CODEWITHSUNDEEP
cstringpointerssegmentation-fault
Leeho Lim
Leeho Lim

Reputation: 19

Segmentation Fault 11 Error Output

I am currently getting a segfault, and I just can't seem to figure out why... I am making a code that concatenates string values:

char* concat(char** strs, unsigned int nstrs)
{
  char* newstring;
  int length = 0;
  int j;
  int charcount;
  int strcount;
  int k = 0;
  for (j = 0; j <= nstrs - 1; j++) {
    length = sizeof(strs[j]) + length;
  }
  newstring = malloc(length);
  for (strcount = 0; strcount <= nstrs - 1; strcount++) {
    for (charcount = 0; charcount <= strlen(strs[strcount]) - 1; charcount++)     {
      newstring[k] = strs[charcount][strcount];
      k++;
    }
  }
  return newstring;

And in my main function I have...

  char* introname[] = {"My", "name", "is", "Trill-o"};
  printf("%s\n", concat(introname, 4));

Upvotes: 0

Views: 55

Answers (5)

a_pradhan
a_pradhan

Reputation: 3295

Also C strings are null terminated character arrays. Make sure the concatenated string has a \0 at the end. Here is a working version : string concatenation

Note I also switched the indexes of the arrays. I suppose this is what you want.

newstring = malloc(length + 1); // for '\0' character
...
newstring[k] = strs[strcount][charcount];
...
newstring[length] = '\0' ;

Upvotes: 0

Sourav Ghosh
Sourav Ghosh

Reputation: 134346

in your code, you need to change

sizeof(strs[j])

to

strlen(strs[j])

Always remember, sizeof is not a function, it's an operator. It returns the size of the supplied data type. In yor code, strs[j] is of type char *, so sizeof will return a value equal to sizeof(char *).

To get the length of the string, you have to use strlen().

That said, please note, strlen() does not include the count for terminating null. So, you've to add space for one more byte while using length in malloc(), like

  newstring = malloc(length + 1);    // 1 more byte for storing the terminating null.

Also, you must check the return value of malloc() to ensure the success. In case if malloc() fails, it will return NULL and the subsequent usage of newstring will lead to UB.

As per the logical part, your code should read

 newstring[k] = strs[strcount][charcount];

and to properly terminate the string,

newstring[k] = '\0' ;

outside for loop.

Upvotes: 2

abligh
abligh

Reputation: 25129

Your main problem is here:

    length = sizeof(strs[j]) + length;

sizeof does not give the required length of the string as it is a char *, not an array. What you want is strlen(strs[j])).

Also, when you've done totalling the lengths, add one for the terminating NUL before you malloc.

Finally this:

  newstring[k] = strs[charcount][strcount];

should be 

  newstring[k] = strs[strcount][charcount];

Upvotes: 0

MichaelCMS
MichaelCMS

Reputation: 4763

Don't use sizeof to get the length of a string.

You need to use strlen.

 sizeof(strs[j]) ; // bad, will return the sizeof pointer which is 4 or 8 depending on the system
 strlen(strs[j]); // this is what you want.

Upvotes: 1

Gopi
Gopi

Reputation: 19864

sizeof(strs[j])

in the function will give sizeof(pointer) not sizeof(array) But since you have a string use strlen(strs[j]) to get the length of the string.

Please make a note to allocate memory to the \0 character also.

Upvotes: 1

Related Questions