How do I dynamically allocate memory for an array of strings in C?

Question

I read the previous questions on dynamic arrays in C however I was not able to relate the answers to my question.

I am taking commands from stdin using fgets, removing the newline character and then want to store each command delimited by a space in a dynamically allocated array of strings. I however am having a lot of trouble with the correct way to allocated and reallocate memory. I am compiling with clang and keep getting segmentation fault 11. I then used -fsanitize=address and keep getting:

==2286==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x60200000eeb8 at pc 0x000108fb6f85 bp 0x7fff56c49560 sp 0x7fff56c49558 WRITE of size 8 at 0x60200000eeb8 thread T0

Here is my code:

// Sets a delimiter to split the input
const char *seperator = " ";

char *token = strtok(line, seperator);

char **cmds = (char **) malloc(sizeof(char) * sizeof(*cmds));
// Adds first token to array of delimited commands
cmds[0] = token;

int count = 1;
while (token != NULL) {
    token = strtok(NULL, sep);
    if (token != NULL) {
        cmds = (char **) realloc(cmds, sizeof(char) * (count + 1));
        // Adds next token array of delimited commands
        cmds[count] = token;
        count++;
    }
}

Possible duplicate of [Segmentation fault but unable to reason how, memory allocation looks fine to me](http://stackoverflow.com/questions/35542391/segmentation-fault-but-unable-to-reason-how-memory-allocation-looks-fine-to-me) — n. m. could be an AI, Apr 14 '16 at 05:02
Don't cast the result of `malloc` & friends in C. And C does not have a string type. It's all just convention. — too honest for this site, Apr 14 '16 at 12:40

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

You're not allocating enough memory. cmds is an array of pointers, so each element is sizeof(char *) bytes, not sizeof(char) bytes.

On the initial allocation you want 1 char *, then on subsequent allocations you want count + 1.

Also, don't cast the return value of malloc, as that can hide other problems, and don't forget to check for failures.

char **cmds = malloc(sizeof(char *) * 1);
if (cmds == NULL) {
    perror("malloc failed");
    exit(1);
}
...
        cmds = realloc(cmds, sizeof(char *) * (count + 1));
        if (cmds == NULL) {
            perror("reallocfailed");
            exit(1);
        }

Basile Starynkevitch · Answer 2 · 2016-04-14T05:51:00.717

First, sizeof(char) is always 1 by definition. And coding that does not make your code more readable.

But a pointer to char needs sizeof(char*) bytes (depending on the machine & ABI, that is often 8 or 4 bytes). I would at least suggest to compile your code with gcc -Wall -Wextra -g if using GCC.

At last, I find your code a bit inefficient. You are calling realloc at every loop. I would maintain a variable containing the allocated size

 int allocsize = 4; // allocated size in number of elements
 char **cmds = malloc(allocsize*sizeof(char*));
 if (!cmds) { perror("malloc"); exit(EXIT_FAILURE); };

^{(BTW, always check the result of malloc; it can fail).}

And to avoid realloc-ing every time, I would grow the allocated size in a geometric fashion, so inside the loop:

 if (count>=allocsize) {
    int newallocsize = (4*allocsize)/3+10;
    cmds = realloc (cmds, newallocsize*sizeof(char*));
    if (!cmds) { perror("realloc"); exit(EXIT_FAILURE); };
    allocsize = newallocsize;
 }

Alternatively, instead of keeping three variables: cmds, count, allocsize you could use a single struct ending with a flexible array member (and keeping its allocated and used sizes).

score 2 · Answer 3 · answered Apr 14 '16 at 05:48

The first malloc is wrong.. What'll you get when you derefer cmd by *cmd, before it's even allocated?
It also uses sizeof(char), which is wrong..

the right way would be..

// strtok modifies the string. So use writable string
char line[80] = "Hello my name is anand";
char *token = strtok(line, sep);
int count = 0;
// Alloc array of char* for total number of tokens we have right now
char **cmds = (char **) malloc(sizeof(char*) * (count + 1));

while (token != NULL) 
{
    /**
     * Alloc memory for the actual token to be stored.. 
     * token returned by strtok is just reference to the existing string 
     * in 'line'
     */
    cmds[count] = malloc(sizeof(char) * ((strlen(token) + 1)));
    // Adds tokens to array of delimited commands
    strcpy(cmds[count], token);
    count++;

    token = strtok(NULL, sep);
    if (token != NULL) 
    {
        // resize array of tokens to store an extra token
        char ** newCmds = (char **) realloc(cmds, sizeof(char*) * (count + 1));
        // only if realloc was successful then use it.
        if (newCmds != NULL)
        {
             cmds = newCmds;
        }
    }
}

How do I dynamically allocate memory for an array of strings in C?

3 Answers3