CODEWITHSUNDEEP
c++linuxsegmentation-faultappendchars
daxvena
daxvena

Reputation: 1150

Segmentation fault when appending two chars - C++

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?

Upvotes: 1

Views: 1374

Answers (9)

Ace
Ace

Reputation: 355

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;

Upvotes: 0

Keith
Keith

Reputation: 6834

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.

Upvotes: 2

Xeo
Xeo

Reputation: 131799

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());
}

Upvotes: 1

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361472

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

Upvotes: 3

Mark Wilkins
Mark Wilkins

Reputation: 41222

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).

Upvotes: 0

Michael Aaron Safyan
Michael Aaron Safyan

Reputation: 95509

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);

Upvotes: 1

Skyler Saleh
Skyler Saleh

Reputation: 3991

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);

Upvotes: 0

templatetypedef
templatetypedef

Reputation: 372814

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!

Upvotes: 2

AK_
AK_

Reputation: 8099

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);

Upvotes: 5

Related Questions