Segmentation fault when appending two chars - C++

1.3k views Asked by At

I am trying to append two chars but for some reason I am getting a segmentation fault.

My code is like;

#include <string.h>
char *one = (char*)("one");
char *two = (char*)("two");

strcat(one, two);

and I seem to be getting a segmentation fault at strcat(one, two), why is that?

9

There are 9 answers

0
AK_ On BEST ANSWER

http://www.cplusplus.com/reference/clibrary/cstring/strcat/

the first parameter to strcat, must be big enough to hold the resulting string

try:

//assuming a,b are char*
char* sum = new char[strlen(a) +strlen(b)+1];
strcpy(sum,a);
strcat(sum,b);
16
Sarfaraz Nawaz On

There should be enough legal memory to hold the entire string.

char *one = new char[128]; //allocating enough memory!
const char *two = "two"; //"two" is const char*

strcpy(one, "one");
strcat(one, two); //now the variable "one" has enough memory to hold the entire string

By the way, if you prefer using std::string over char* in C++, such thing would be easier to handle:

#include<string>

std::string one = "one";
std::string two = "two";

one = one + two; //Concatenate 

std::cout << one;

Output:

onetwo
0
Mark Wilkins On

strcat needs a "writeable" buffer as the target. In your example, it is a pointer to a string constant (or literal), which you cannot write to, thus it results in an exception. The target buffer can be a buffer on the stack or one dynamically allocated (e.g., with malloc).

5
Xeo On

You never reserved some space for your strings.

#include <string.h>
#include <stdio.h>

int main(void){
    char str[20] = "";
    strcat(str, "one");
    strcat(str, "two");
    printf("%s", str);
}

Would be one correct way to do this. The other (and way better) is to use the std::string class.

#include <string>
#include <cstdio>

int main(void){
    std::string str;
    str += "one";
    str += "two";
    std::printf("%s", str.c_str());
}
0
templatetypedef On

There are two reasons for this.

  1. If you have a pointer initialized to a string literal, that memory is read-only and modifying it will result in undefined behavior. In this case, if you try to append a string to a string literal, you'll be modifying this sort of memory, which will result in problems.

  2. When using strcat you need to guarantee that space exists for the concatenation of the string at the location you're specifying. In this case, you cannot guarantee that, since a string literal is only guaranteed to have enough space to hold the literal itself.

To fix this, you'll want to explicitly allocate a buffer large enough to hold the concatenation of the two strings, including the null terminator. Here's one approach:

char* buffer = malloc(strlen(one) + strlen(two) + 1);
strcpy(buffer, one);
strcat(buffer, two);

Hope this helps!

1
Skyler Saleh On

Your destination string should be large enough to hold both the destination and the source string. So an example would be

char one[10] = "one";
char two[4] = "two";
strcat(one,two);

0
Michael Aaron Safyan On

There are several problems here. Firstly, though you have casted the strings to mutable versions, they really are string literals and hence should not be written. Secondly, you are using strcat which will write into the string buffer, completely ignoring the length of the string buffer (it's better to use strncat which requires you to specify the length of the buffer). Lastly, since this is C++, it would be way better to use:

#include <string>

// ...

string one = "one";
string two = "two";
one.append(two); 
0
Keith On

The seg fault is because you attempt to write to read only memory. The first action of the strcat is to copy of 't' from the first entry of two into the null at the end of "one". So strictly the seg fault is not due to lack of storage - we never get that far. In fact this code will also likely give you a seg fault:

char* one = "one";
char* two = "";
strcat(one, two);    

All this tries to do is copy a null over a null, but in read-only memory. I suppose a optimiser might happen to stop this on some platforms.

Oddly enough the following (incorrect) code will (probably) not give you a seg fault, and even give the "right" answer:

char one[] = "one";
char two[] = "two";
strcat(one, two);   
printf("%s\n", one);

This successfully writes "onetwo" to stdout on my machine. We get a stack scribble, which we happen to get away with.

On the other hand this does seg fault:

char* one = "one        "; // Plenty of storage, but not writable.
char two[] = "two";
strcat(one,two);    

Hence the solution:

const unsigned enoughSpace = 32;
char one[enoughSpace] = "one";
char two[] = "two";
strcat(one,two);    
printf("%s\n", one);

The issue with this is of course, how large to make enoughSpace in order to store what ever is coming?

Hence the functions strncat, or strcat_s, or more easily std::string.

Moral of the story: in C++, just like C, you really need to know what your memory layout is.

0
Ace On

This is not the problem of "not enough space".

char *a = "str";

Look at the code above, the pointer a is point to "static memory". The string "str" is stored in the static place in the PCB, which means it can't be overwritten.

So, the codes below will be better:

#include <string>
using std::string;

string a = "stra";
string b = "strb";

a += b;