Incorrect 2D array access

Question

I've got an unusual condition. Here's the snippet:

int i, j;
short ** s = (short **)malloc(128);
for(i = 0; i < 14; i++){
    s[i] = (short *)malloc(128);
    for(j = 0; j < 128; j++)
        s[i][j] = 0;
}
printf("Value of s[%d][%d] = %d\n",2,40,s[2][40]);
s[1][108] = 99;
printf("Value of s[%d][%d] = %d\n",2,40,s[2][40]);

The output I get when I run this is:Value of s[2][40] = 0 Value of s[2][40] = 99

Eliminating the loops and writing short s[14][128] yields the correct output (Value of s[2][40] is 0 in both prints)

Why am I able to access s[2][40] with s[1][108]? I'm using gcc on Ubuntu 12.04.

ot: even if you have a loop with a single statement, you should always put braces (good practice) — Luca Davanzo, Sep 03 '14 at 11:26
[Please don't cast the return value of `malloc()` in C](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). — unwind, Sep 03 '14 at 11:29

score 6 · Accepted Answer · answered Sep 03 '14 at 11:27

You are accessing out of bounds of the space you allocated, which causes undefined behaviour.

s[i] = (short *)malloc(128) allocates 128 bytes. But then you try to write 128 shorts to that space. Since shorts are bigger than 1 byte, you run off the end of the allocation.

Also, short ** s = (short **)malloc(128); probably allocates too much space, since you only use 14 rows.

Assuming you want an array of 14 x 128 the way is:

short **s = malloc( 14 * sizeof *s );
for (size_t i = 0; i < 14; ++i)
    s[i] = malloc( 128 * sizeof *s[i] );

malloc takes the number of bytes, so you have to multiply the number of elements by the size of each element.

Note that if you do not need the feature of being able to make different rows different sizes then you can allocate it in a single bloc:

short (*s)[128] = malloc( 14 * sizeof *s );

and in both cases you can use s[i][j] the same.

Finally, you should also check that the pointer returned by malloc is not NULL, and also free the pointer after you are finished with the memory.

That's right. I forgot that malloc takes the number of bytes and a short is 2 bytes on my system. Thanks a lot. — PrithviJC, Sep 03 '14 at 16:46

score 0 · Answer 2 · answered Sep 03 '14 at 12:01

First it is not a correct way of allocating memory for 2D array-

short ** s = (short **)malloc(128); 
for(i = 0; i < 14; i++){
    s[i] = (short *)malloc(128);

This is wrong way!

 short ** s = (short **)malloc(128);

Here you are allocating memory for array of 128 bytes for short **.

for(i = 0; i < 14; i++){
    s[i] = (short *)malloc(128);

But you are using only 14 short *. That is only 28 bytes used(because sizeof(short) = 2).

Why am I able to access s[2][40] with s[1][108]?- Because you are accessing the array out of bounds!

How?

s[i] = (short *)malloc(128);

This will allocate memory for array of 128 bytes for each short *. That means you can store Maximum 64 short elements in Each 1D array.

But you are trying to access s[1][108] - means 108th element in the 1st row of the array. It is Out of bounds for the allocated memory. So you will not get the expected output.

Try to allocate memory like this-

short **s = malloc(sizeof(short *) * 14);
for(i = 0; i < 14; i++){
    s[i] = malloc(sizeof(short) * 128);

Incorrect 2D array access

2 Answers2