Segfault when initializing 2d array

Question

I cannot find what's wrong in this simple array initialization. The program crashes with a segfault on field[x][y] = ' ';, x and y at 0 (I use Code::Blocks debugger)

/* init data structures */
char **field;
int field_width=5,field_height=5;
field = malloc(sizeof(char*)*field_width);
for(x=0;x<field_width;x++)
{
    field[x] = malloc(sizeof(char)*field_height);
    for(y=0;y<field_height;y++)
    {
        field[x][y] = ' ';
    }
}

Any idea of what I am doing wrong ?

This code snippet doesn't crash for me. – Neil Oct 12 '12 at 23:21 — Neil, Oct 12 '12 at 23:21

score 1 · Answer 1 · answered Oct 12 '12 at 23:15

1

field = (char*) malloc(sizeof(char*)*field_width);

The char* cast maybe?

answered Oct 12 '12 at 23:15

Science_Fiction

3,403
23
27

1

In (ANSI) C, there is no need to cast the result of malloc. See http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc – reima Oct 12 '12 at 23:23
1

Unlike C++, you don't need to explicitly cast from `void*` to another pointer in C. – Neil Oct 12 '12 at 23:23

score 1 · Accepted Answer · answered Oct 12 '12 at 23:34

1

I actually simplified the code snippet. field_width was not initialzed. I'm surprised this did not raise a warning during the build. And I don't really know why it generates a segfault when x=0.

But my problem is solved. Thank you all and sorry for the conveniance...

answered Oct 12 '12 at 23:34

Arnaud Courtecuisse

369
2
13

With an uninitialised `field_width`, that could have happened to have a large value, so that the `malloc` failed. Did you check the return value of `malloc` in the real code? – Daniel Fischer Oct 13 '12 at 11:16
I didn't check. Following your advice I did, and indeed, that is what failed. Thanks for the advice. Another good habit I have to remember! – Arnaud Courtecuisse Oct 14 '12 at 21:50

nick_w · Answer 3 · 2012-10-12T23:30:36.343

0

Shouldn't it be this?

field = (char**)malloc(sizeof(char*)*field_width);

Edit

malloc can return null, so it would pay to check that field[x] = malloc(sizeof(char)*field_height); block of memory is valid.

edited Oct 12 '12 at 23:30

answered Oct 12 '12 at 23:21

nick_w

14,758
3
51
71

1

Unlike C++, you don't need to explicitly cast from `void*` to another pointer in C. – Neil Oct 12 '12 at 23:23

score 0 · Answer 4 · edited May 23 '17 at 11:43

Before you initialized field_width, it probably contained random data. Using field_width in the malloc statement then triggered undefined behavior. The compiler could do whatever it wanted, from skipping the malloc to using whatever garbage happened to be stored in field_width, or even worse/stranger things!. Regardless, you were unlikely to get the malloc call you wanted, and either if it didn't run or returned NULL (e.g. if field_width contained a value that was too large to be malloced), the resulting value of field was unlikely to point to valid memory. This would then cause a segfault when you dereference field in the loop. You were fortunate that you got such a clear sign that something was wrong -- memory errors aren't always so blatant.

Segfault when initializing 2d array

4 Answers4