Segmentation fault when using sprintf in dynamically allocated array

Question

I am converting integers to strings and adding them to a dynamically allocated array. The problem is that it is causing a segfault. I don't understand why it is happening.

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

int main() {
    char *x = malloc(10 * sizeof(char));
    x[0] = malloc(10 * sizeof(char));
    sprintf(x[0],"%d",10);
    
    for(int i = 0; i < 10;i++){
        free(x[i]);
    }
    
    free(x);
    return 0;
}

Answer 1

The best way to determine the amount of memory to be used with malloc is this:

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

#define N_STRINGS 10
#define STRING_SZ 10

int main() {
    // if you use *x (the deferred subexpression) the compiler can calculate its
    // sizeof easily, and no need to use a constant or something that has to be
    // revised if you change the type of x.  Also, calloc will give instead N_STRINGS
    // pointers already initialized to NULL.
    char **x = calloc(N_STRINGS, sizeof *x);

    // to be able to free(x[i]) for all i, you need to initialize all pointers,
    // and not only the first one.
    int i; // I prefer this, instead of for(int i..., which is more portable with legacy code.
    for (i = 0; i < N_STRINGS; i++) {
        // char is warranted to be sizeof 1, you don't need to specify but the
        // number of chars you want for each character array.
        x[i] = malloc(STRING_SZ); // remember only STRING_SZ chars you have, so...
        // better to use snprintf(), instead.
        snprintf(x[i], // the buffer pointer
                 STRING_SZ,   // the buffer size (STRING_SZ chars, incl. the final null char)
                 "%d", // the format string
                 10);  // initialize all strings to the string "10" ???
    }
    // upto here we have made N_STRINGS + 1 calls to malloc...

    // you should do something here to check that the allocations went fine, like
    // printing the values or do some work on the strings, but that's up to you.
    
    // now, everything should be fine for free() to work.
    for(int i = 0; i < N_STRINGS; i++){
        free(x[i]);
    }
    
    free(x); // ... and this is the N_STRINGS + 1 call to free.
    return 0;
}

Check always that the number of free calls executed by your program has to be the same of the malloc calls you have made (before). A free call must free the memory allocated by one (and only one) call to malloc. You cannot free something that has not been acquired by malloc() (or calloc()) directly or indirectly. The same as it is bad use (but not necessary an error) to do a malloc that is not freed before the program ends (this is not true in non-hosted environments, e.g. embedded programs, in which the operating system doesn't deallocate the memory used by a program when it finishes, although)

By the way, the reason of your segmentation fault is precisely that you have made only two calls to malloc(), but you made 11 calls to free(). free() tries to free memory that malloc() has not allocated, or even worse, you don't own. Anyway, this drives you to Undefined Behaviour, which is something you don't desire in a program. In this case, you got a program crash.

Answer 2

If you want a dynamic allocated array of strings, you should declare your variable x as a pointer to an array of e.g. 32 chars. The you can allocate/deallocate an array of these using a single malloc and likewise a single free.

Like:

#define NUM_STRINGS 10
#define STRING_SIZE 32

int main() {
  // declare x as a pointer to an array of STRING_SIZE chars
  char (*x)[STRING_SIZE];

  // Allocate space for NUM_STRINGS of the above array, i.e.
  // allocate an array with NUM_STRINGS arrays of STRING_SIZE chars
  x = malloc(NUM_STRINGS * sizeof *x);
  if (x)
  {
    for (int i = 0; i < NUM_STRINGS; ++i)
    {
      sprintf(x[i], "%d", 10 + i);
    }

    for (int i = 0; i < NUM_STRINGS; ++i)
    {
      puts(x[i]);
    }

    free(x);
  }

  return 0;
}

Output:

10
11
12
13
14
15
16
17
18
19

Answer 3

To allocate an array whose elements are char*, the pointer to point the array should be char**, not char*.

Also you mustn't use values in buffer allocated via malloc() and not initiaized. The values are indeterminate and using them invokes undefined behavior.

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

int main() {
    /* correct type here (both variable and allocation size) */
    char **x = malloc(10 * sizeof(char*));
    x[0] = malloc(10 * sizeof(char));
    sprintf(x[0],"%d",10);

    /* initialize the other elements to pass to free() */
    for (int i = 1; i < 10; i++) x[i] = NULL;
    
    for(int i = 0; i < 10;i++){
        free(x[i]);
    }
    
    free(x);
    return 0;
}