2

I wanted to use strcat() to concatenate an element of an array of strings. I tried:

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

int main() {
    char **str = malloc(sizeof(char *) * 3);

    for (int i = 0; i < 3; i++) {
        str[i] = malloc(sizeof(char) * 8);
    }

    str[0] = "foo";
    str[1] = "bar";

    strcat(str[0], "H");

    for (int i = 0; i < 3; i++) {
        printf("%s\n", str[i]);
    }

    free(str);

    return 0;
}

and I get the error:

Segmentation fault (core dumped)

What should I do to get it right?

Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335
TechTycho
  • 134
  • 1
  • 10
  • 3
    The `str[0] = "foo"; str[1] = "bar";` overwrites the memory allocation pointers, and you consequently try to concatenate to a string literal: so a segfault. Use `strcpy()` to copy a string. – Weather Vane Dec 19 '21 at 18:57
  • 2
    One note: you `free` `str` but not the 8 bytes of memory each of those three pointers points to. Given that this in main and it's a tiny amount of memory, this is probably not a practical concern, but it's good to get into good habits wrt memory management. – Chris Dec 19 '21 at 18:58
  • 2
    The `printf("%s\n", str[2]);` will be undefined behaviour as the allocated memory doesn't contain a string. – Weather Vane Dec 19 '21 at 19:00
  • 1
    Or to add to what @WeatherVane suggested, you may wish to use `strncpy` to _ensure_ you don't have a buffer overflow. Again, not a practical concern here, but good habits... – Chris Dec 19 '21 at 19:00
  • 2
    Note that strictly this isn't a "2d array" but a "jagged array" or "array of arrays". – Weather Vane Dec 19 '21 at 19:09

1 Answers1

2

For starters the program has memory leaks. At first memory was allocated and its addresses were stored in pointers str[i]

for (int i = 0; i < 3; i++) {
    str[i] = malloc(sizeof(char) * 8);
}

and then the pointers str[0] and str[1] were reassigned by addresses of string literals.

str[0] = "foo";
str[1] = "bar";

As a result you may not change a string literal by this statement

strcat(str[0], "H");

because this invokes undefined behavior

You have to write

strcpy( str[0], "foo" );
strcpy( str[1], "bar" );

And this loop

for (int i = 0; i < 3; i++) {
    printf("%s\n", str[i]);
}

also invokes undefined behavior because the element str[2] was not initialized by a string.

Either you need to change the condition or the loop like i < 2 or to initialize the element str[2].

And you need to free all the allocated memory like

for (int i = 0; i < 3; i++) {
    free( str[i] );
}

free( str );
Vlad from Moscow
  • 301,070
  • 26
  • 186
  • 335