C: Segfault when writing string concatenating function

Question

I'm trying to write a string concatenating in C. I'm not allowed to use strcat. My program compiles without errors. But when I run it, I get a segfault. Can someone tell me what's wrong? Thanks. Here's my code

char* concat(char** strs, unsigned int nstrs)
{
  unsigned int outlen = 0;
  char* output;
  int x;
  int y;

  for(x = 0; x < nstrs ; x++) {
    outlen += strlen(strs[x]);
   }

  output = (char *) malloc(outlen * sizeof(char));

  for(x = 0; x < nstrs ; x++) {
    for(y = 0 ; y < strlen(strs[x]) ; y++) {
     output[x+y] = strs[x][y];
}  
}

      return output;
}


int main(int argc, char *argv[])
{
  int strsize = 0;
  int x;

  for(x = 0; x < argc; x++) {
    strsize += strlen(argv[x+1]);
  }

  char** strs;
  strs = (char* *) malloc(sizeof(char)*strsize);

  for(x = 0; x < argc; x++) {
    strs[x] = argv[x+1];
  }

  concat(strs, (argc - 1));

  printf("%s", concat(strs, (argc -1)));

  free(strs);

  return 0;
}

``output[x+y]`` will create surprising output ;) Better have a ``writePosition`` variable you increment each time you write a char to output. — BitTickler, Feb 02 '15 at 05:48

Gopi · Accepted Answer · 2015-02-02T05:56:25.810

0

strs = (char* *) malloc(sizeof(char)*strsize);

should be

strs = malloc(sizeof(char *)* <num of pointers>);

strsize should include \0 character in each string.

Even if you fix this there is array out of bound access

  for(x = 0; x < argc; x++) {
    strsize += strlen(argv[x+1]);
  }

edited Feb 02 '15 at 05:56

answered Feb 02 '15 at 05:46

Gopi

19,784
4
24
36

score 0 · Answer 2 · edited May 23 '17 at 12:12

The problem here is

strsize += strlen(argv[x+1]);

When x is 1 less than argc, then by saying argv[x+1], you're accessing out of bound memory [Remember, array indexing starts from 0].

Also, please do not cast the return value of malloc().

Then again, the memory allocation logic for char** strs; is faulty. IMO, what you need is something like

strs = malloc(sizeof(char*) * (argc-1));

Lastly, as other answers suggested, you need to allocate memory for output as one more byte than that of the source string length calculated using strlen() as strlen() does not count the terminating null required for a string representation.

score 0 · Answer 3 · answered Feb 02 '15 at 05:52

strs = (char* *) malloc(sizeof(char)*strsize);

Your stars don't add up. If you want to allocate an array of char*, why are you using sizeof(char)?

output = (char *) malloc(outlen * sizeof(char));

Here your sizeof is right, but you are forgetting about the NUL termination of strings. You need to allocate one extra character, and then set it to '\0'.

C: Segfault when writing string concatenating function

3 Answers3