Initializing a big array of a certain structure -- getting seg faults

Question

The following code is giving me a seg fault. gdb says its coming from the line memos[j][k] -> cost = -1;, but I can't tell exactly what is wrong. I'm guessing something with how I am allocating memory and that the array index is mistakenly out of bounds?

memo_array_t mk_empty_memo_array(int l) {

    int n = pow(2, l+2);
    memo_t **memos = (memo_t **) malloc(n*sizeof(struct memo_s));
    for(int i = 0; i < pow(2, l+2); i++) {
        memos[i] = (memo_t *) malloc(n*sizeof(struct memo_s *));
    }
    for (int j = 0; j < n; j++) {
        for (int k = 0; k < pow(2, l+2); k++) {
            memos[j][k] -> cost = -1;
            memos[j][k] -> color = -1;
            memos[j][k] -> split = -1;
            memos[j][k] -> box = NULL; 
            memos[j][k] -> split_value = -1; 
        }
    }

    memo_array_t memo_array = (memo_array_t) malloc(sizeof(struct memo_array_s));
    memo_array -> dim = n;
    memo_array -> memos = memos;

    return memo_array;
}

In case you want to see the struct typedefs:

typedef struct memo_s {
  int cost;
  int color;
  int split;
  double split_value;
  box_t box;  
} *memo_t;

typedef struct memo_array_s {
  int dim;
  memo_t **memos;
} *memo_array_t;

First of all, [don't cast the result of `malloc` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858). Then you're mixing the types for the `sizeof` operator in your two calls. The first call allocates an array of *pointers* and the second an array of *values*. Or rather, the first call should use a double-pointer to the structure, or a pointer to `memo_t` as operand to the `sizeof` operator. — Some programmer dude, Mar 19 '15 at 20:23

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

It should be:

memo_t **memos = malloc(n*sizeof(struct memo_s **));
for(int i = 0; i < n; i++) {                // ^^ two *s here!
    memos[i] = malloc(n*sizeof(struct memo_s *));   

    // And finally, allocate the actual structs:  
    for(int j = 0; j < n; j++) {    
        memos[i][j] = malloc(sizeof(struct memo_s); 
    }
}

So the first malloc allocates an array of pointers to pointers, the second malloc allocates an array of pointers, and the third malloc (which you were missing) allocates space for the actual structs (since you had a 2D array of pointers, not a 2D array of structs).

Also, don't cast the result of malloc. And in the loop condition, use the value of n you calculated instead of calculating it again.

I think it would be simpler to have just one name for the struct and then add a * after the name to make it a pointer, instead of having memo_s be the actual struct and memo_t being a pointer to that struct. I found it really confusing while trying to fix this.

Ok, I made the suggested changes. Unfortunately, I still receive the same error (segfault at the memos[x][y] -> cost = -1). I fixed the casting problem too. — Craig, Mar 19 '15 at 20:34
Wait a sec I'm not done with all your errors yet... I'll edit the answer soon. Check Joachim's answer meanwhile. — Emil Laine, Mar 19 '15 at 20:34

score 1 · Answer 2 · answered Mar 19 '15 at 20:30

1

Besides your allocations of the arrays being (partly) wrong, the problem is that you don't allocate the actual structures. memo[j][k] is a pointer, but you don't make it point anywhere so when you dereference it you have undefined behavior.

answered Mar 19 '15 at 20:30

Some programmer dude

400,186
35
402
621

I guess I'm confused by your suggestion -- it seems like the first for loop in my code is allocating space for memo[j][k] – Craig Mar 19 '15 at 20:45
1

@Craig You have, in a way, an array of arrays of pointers. You allocate the outer and inner array, but not the contained pointer. See the answer from Zenith. – Some programmer dude Mar 19 '15 at 20:59

Initializing a big array of a certain structure -- getting seg faults

2 Answers2