views:

311

answers:

5

The code below compiles, but immediately crashes for reasons obvious to others, but not to me. I can't seem to get it right, can anyone tell me how to fix this.

*array_ref[2] = array[0];
*array_ref[3] = array[1];

It crashes on that part everytime.

typedef struct test {
    char *name;
    char *last_name;
} person;



int setName(person ** array, person ***array_ref) {

    *array = malloc (5 * sizeof(person));
    *array_ref= malloc(5 * sizeof(person*));

   array[0]->name = strdup("Bob");
   array[1]->name = strdup("Joseph");
   array[0]->last_name = strdup("Robert");
   array[1]->last_name = strdup("Clark");


*array_ref[2] = array[0];
*array_ref[3] = array[1];


    return 1;
}



int main()
{
    person *array;
    person **array_r;

   setName(&array,&array_r);

    printf("First name is %s %s\n", array[0].name, array[0].last_name);
    printf("Second name is %s %s\n", array_r[3]->name, array_r[3]->last_name);

     while(1) {}
    return 0;
}
A: 

You allocated space for the array_ref pointers, but not for what they point to.

Richard Pennington
He points them to objects which are allocated, this is not a problem.
Roger Pate
+1  A: 

Try changing the following in setName()

 *array_ref[2] = array[0];
 *array_ref[3] = array[1];

to

*(*array_ref+2) = array[0];
*(*array_ref+3) = array[1];

This works.

Sean A.O. Harney
+1  A: 

array[1]->name is your problem. This should be (*array)[1].name. Notice how the two aren't equivalent. All the similar uses have the same problem, except for [0], which accidentally does the right thing.

Remember that array, the function parameter, isn't your array, it's a pointer to your array.

Roger Pate
+6  A: 

Operator [] has higher precedence than unary operator*. Hence, this:

*array_ref[2] = array[0];
*array_ref[3] = array[1];

actually means:

*(array_ref[2]) = array[0];
*(array_ref[3]) = array[1];

Types are correct here, which is why it compiles. But from your code it's clear that your intent actually was:

(*array_ref)[2] = array[0];
(*array_ref)[3] = array[1];

So just use parentheses.

Pavel Minaev
Remember `array[1]` is not allocated :).
Simon Buchan
+1  A: 

In functions like this I prefer code like:

int setName(person ** out_array, person ***out_array_ref) {
    person* array = malloc(5 * sizeof(person));
    person** array_ref = malloc(5 * sizeof(person*));
    array[0].name = strdup("Bob");
    array[1].name = strdup("Joseph");
    array[0].last_name = strdup("Robert");
    array[1].last_name = strdup("Clark");
    // I'm guessing this was your intent for array_ref, here:
    array_ref[2] = &array[0];
    array_ref[3] = &array[1];

    *out_array = out_array;
    *out_array_ref = array_ref;
    return 1;
}

Note that this catches both array[1]->name as noted by Roger Pate, and *array_ref[2] = array[0] as (almost) noted by Pavel - whose solution (*array_ref)[2] = array[0] assigns from an unallocated person* array[1] - both of which are hard to notice with the extra dereference.

Of course, I mostly do this because I use C++, and this increases exception safety ;).

Simon Buchan