Why does this dynamic allocation of type char** and char* using malloc segfault?

Question

I don't understand why this code segmentation faults. It can work if I define a char** inside of the function, allocate to that char**, then point *commandsArray at that char**. Can someone explain what I am not understanding? Thanks in advance.

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

void input_str_to_sngl_commands( char*** commandsArray );

int main()
{

    char** commandsArray_var;
    input_str_to_sngl_commands( &commandsArray_var );

return 0;
}

void input_str_to_sngl_commands( char*** commandsArray )
{
    *commandsArray = (char**) malloc(2*sizeof(char**));
    *commandsArray[0] = (char*) malloc(30*sizeof(char));
    *commandsArray[1] = (char*)malloc(30*sizeof(char));
}

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

You got the precedence wrong: [] has higher precedence than *, so *commandsArray[1] accesses a wrong address.

Use parentheses to force evaluation order, like this

*commandsArray = malloc(2*sizeof(char*));
(*commandsArray)[0] = malloc(30*sizeof(char));
(*commandsArray)[1] = malloc(30*sizeof(char));

or use a temporary variable to use a more readable syntax:

char** ret = malloc(2*sizeof(char*));
ret[0] = malloc(30*sizeof(char));
ret[1] = malloc(30*sizeof(char));
*commandsArray = ret;

Demo.

Note: Casting malloc is unnecessary.

score 0 · Answer 2 · answered Apr 10 '15 at 00:35

*commandsArray[1] is the same as *(commandsArray[1]), but what you wanted here was (*commandsArray)[1].

commandsArray[1] is the memory after the commandsArray_var (which contains garbage as far as you are concerned), treated as a char*.

*commandsArray[1] tries to dereference the garbage char*, which segfaults.

All you need to do is add parentheses - make it (*commandsArray)[1].

This also affects the previous line, which uses *commandsArray[0], but coincidentally (since *x == x[0]), (*commandsArray)[0] is the same as *(commandsArray[0]) (and both are the same as **commandsArray). You should add the parentheses to that line as well anyway, to make it clear what your code is trying to do.

score 0 · Answer 3 · edited May 23 '17 at 11:50

*commandsArray[0] should be (*commandsArray)[0].

Also, you malloc the wrong amount of space. The chance of making this mistake can be reduced by using a sizeof expression that corresponds to the type being pointed to by the pointer you are creating, as explained here.

Using a temporary pointer as suggested by dasblinkenlight is a great idea too. This makes it easier to clean up from allocation failure and easier to read your code:

char **new;

new = malloc( 2 * sizeof *new );
new[0] = malloc( 30 * sizeof **new );
new[1] = malloc( 30 * sizeof **new );

*commandsArray = new;

Why does this dynamic allocation of type char** and char* using malloc segfault?

3 Answers3