Why Isn't malloc() Allocating Enough Memory

Question

I'm having some trouble understanding the functionality of malloc and perhaps that's why I'm seeing this issue, but hopefully someone here can help me understand.

I am allocating a 2D array using the following function:

int16_t** create_2d_array(uint8_t num_rows, uint16_t num_cols){
    uint8_t i = 0;  

    int16_t **arr = (int16_t **)malloc(num_rows * sizeof(int16_t *));

    for(i=0; i<num_rows; i++) {
        arr[i] = (int16_t *)malloc(num_cols * sizeof(int16_t));
    }               

    return arr;     
}

I am calling this function like so:

twoD = create_2d_array(4, 512);

If I stop my code at the beginning of the for loop and check the contents of arr using my GDB terminal, then I get the following:

gdb $ p/x arr[0]
$96 = 0x2001fa00

gdb $ p/x arr[1]
$97 = 0x0

gdb $ p/x arr[2]
$98 = 0x0

gdb $ p/x arr[3]
$99 = 0x0

This implies, at least to me, that arr[0] was allocated properly, but not the other elements of arr.

Isn't malloc supposed to determine if the size requested can be allocated and if not, then it should return a NULL pointer? At least that's my understanding of malloc. Is there something I'm doing wrong?

As a test, I executed the following line:

twoD_temp = create_2d_array(2, 4);

Again I stop the execution at the beginning of the for loop and print the contents of arr.

gdb $ p/x arr[0]
$121 = 0x2001fa00

gdb $ p/x arr[1]
$122 = 0x2001fa10

This is what I would expect. First index is a valid pointer, and the second index is also a valid pointer since I created an array of pointers.

After the for loop executes I print the same contents:

gdb $ p/x arr[0]
$125 = 0x2001fa00

gdb $ p/x arr[1]
$126 = 0x2001fa10

This is still the same which is what I would expect. The only difference now is that there is memory allocated for the columns.

Answer 1

After just the first malloc, arr is a pointer to a chunk of memory that contains nothing but junk. The for loop sets the individual entries to point to the rows.

So neither arr[0] nor arr[1] should contain any particular value until the for loops sets their values to point to the various rows that it creates.

Let's look at the code carefully:

int16_t **arr = (int16_t **)malloc(num_rows * sizeof(int16_t *));

This allocates one block of memory, large enough to hold one pointer for each row. The variable arr will point to this memory. The block of memory contains junk.

for(i=0; i<num_rows; i++) {
    arr[i] = (int16_t *)malloc(num_cols * sizeof(int16_t));
}     

This sets arr[0] to point to a block of memory large enough to hold a row. Until this for loop executes, arr[0] and the other entries in the array just contain junk.

---

Perhaps a diagram will help. After the first allocation before the loop (of arr, the block of ponters), this is what you have:

       +--------+
arr -> | arr[0] | -> points to some arbitrary location
       +--------+
       | arr[1] | -> points to some arbitrary location
       +--------+
       | arr[2] | -> points to some arbitrary location
       +--------+
       | arr[3] | -> points to some arbitrary location
       +--------+

Those pointers will point to arbitrary locations because malloc allocates the memory but does not initialise it to anything.

And that's the state where you're examining everything, so each arr[] can be any value. Once you go through the loop once, you have:

       +--------+    +----------------+
arr -> | arr[0] | -> | arr[0][0..511] |
       +--------+    +----------------+
       | arr[1] | -> points to some arbitrary location
       +--------+
       | arr[2] | -> points to some arbitrary location
       +--------+
       | arr[3] | -> points to some arbitrary location
       +--------+

It's only at that point do your second level allocations begin to point at something useful.

Answer 2

Your code doesn't have a problem. The issue is your understanding of how malloc works.

When memory is allocated dynamically via malloc, the values of the bytes are indeterminate, so they could take any value including 0. They are not guaranteed to contain any particular value.

In your example:

int16_t **arr = (int16_t **)malloc(num_rows * sizeof(int16_t *));

If we assume a pointer takes up 8 bytes and that num_rows is 4, this allocates 32 bytes of space, enough for 4 values of type int16_t *. At this point each member of the array contains no meaningful value. When you later do this:

arr[i] = (int16_t *)malloc(num_cols * sizeof(int16_t));

You assign (assuming a successful call) a valid memory address returned from malloc to a member of the array, overwriting whatever garbage value happened to be there previously. If you were to look at the elements of arr[i] at this point, i.e. arry[i][0], arry[i][1] etc. these values are also indeterminate until you assign something to them.

Answer 3

You've already accepted an answer (and rightly so since it explains in detail what was wrong). I know this because I contributed to it :-)

However, I'd also like to suggest another method of allocating your 2D arrays. Doing it as a series of allocations means that you are responsible for cleaning up the individual allocations when you're done with the array.

Although this can be done with another function, you have to pass the number of rows into that so you can correctly free each row allocation before freeing the pointer array allocation.

A method I've used in the past is to allocate one block of memory big enough for both the pointer array and all the row arrays, then massage it so that it looks like a normal 2D array (so that things like array[row][col] = 7 still work).

The code below does this and the only requirement is that the alignment requirements for int16_t are not more restrictive than that of int16_t* (this would be rare and you could work around it, but it's probably not necessary in the vast majority of environments):

#include <stdint.h>
#include <stdlib.h>
#include <string.h>
#include <malloc.h>

void *create_2d_array(uint8_t num_rows, uint16_t num_cols, int clear_data) {
    // Create a single block big enough for both level-1 pointer array
    // and level-2 value array.

    size_t totalMemSz =
        (num_rows * sizeof(uint16_t*)) +            // pointers.
        (num_rows * num_cols * sizeof(uint16_t));   // values.
    void *memBlock = clear_data ? calloc(totalMemSz, 1) : malloc(totalMemSz);
    if (memBlock == NULL) return NULL;

    // Populate the level-1 pointers to point at the level-2 rows.

    for (size_t i = 0; i < num_rows; ++i) {
        ((int16_t**)memBlock)[i] = (int16_t*)(&(((char*)memBlock)[
            num_rows * sizeof(uint16_t*) +    // skip pointers.
            i * num_cols * sizeof(uint16_t)   // select row.
        ]));
    }

    return memBlock;
}

#include <stdio.h>
#include <stdbool.h>

void dumpArr(int16_t **addr, size_t num_rows, size_t num_cols) {
    for (size_t i = 0; i < num_rows; ++i) {
        printf("Pointer[%zd] = %p, data =", i, addr[i]);
        for (size_t j = 0; j < num_cols; ++j) {
            printf(" %2d", addr[i][j]);
        }
        putchar('\n');
    }
}

int main() {
    puts("Running ...");
    int16_t **arr = create_2d_array(4, 7, true);
    arr[0][0] = 1;
    arr[1][2] = 42;
    arr[3][6] = 77;
    dumpArr(arr, 4, 7);
    free(arr);
}

That code is a complete test program for the create_2d_array function so you can test it how you wish. The important thing is that, when you're finished with the 2D array, you simply release it with a free(arr) rather than having to do any special size-dependent processing.

A sample run of the code as it currently stands:

Running ...
Pointer[0] = 0x675440, data =  1  0  0  0  0  0  0
Pointer[1] = 0x67544e, data =  0  0 42  0  0  0  0
Pointer[2] = 0x67545c, data =  0  0  0  0  0  0  0
Pointer[3] = 0x67546a, data =  0  0  0  0  0  0 77

Answer 4

There's nothing that's really wrong with your code. I have a few remarks that I will take below.

Isn't malloc supposed to determine if the size requested can be allocated and if not, then it should return a NULL pointer?

Yes, that is true. But you're not testing any return values, so how would you know?

Also, it's a good habit to use the variable instead of the type for sizeof. That reduces code duplication. So write T *t= malloc(n * sizeof(*t)) instead of T *t = malloc(n * sizeof(T)). And casting is completely unnecessary and provides not benefits, unless you are compiling your C code with a C++ compiler.

So with all that in mind, I would write the code like this:

int16_t** create_2d_array(uint8_t num_rows, uint16_t num_cols)
{
    uint8_t i = 0;  

    int16_t **arr = malloc(num_rows * sizeof(*arr));
    if(!arr) {
        fprintf(stderr, "Error allocating memory\n");
        exit(EXIT_FAILURE);
    }    
    for(i=0; i<num_rows; i++) {
        arr[i] = malloc(num_cols * sizeof(*arr[0]));
        if(!arr[i]) {
            fprintf(stderr, "Error allocating memory\n");
            exit(EXIT_FAILURE);
        }    
    }        

    return arr;     
}

And if you want to go really hardcore and be able to test your own function the same way you test malloc, then you can do like this. It's one of the few accepted uses for goto

int16_t** create_2d_array(uint8_t num_rows, uint16_t num_cols)
{
    uint8_t i = 0;

    int16_t **arr = malloc(num_rows * sizeof(*arr));
    if(!arr)
        goto cleanup1;

    for(i=0; i<num_rows; i++) {
        arr[i] = malloc(num_cols * sizeof(*arr[0]));
        if(!arr[i])
            goto cleanup2;
    }

    return arr;

cleanup2:
    do {
        free(arr[i]);
    } while(i-- > 0);

cleanup1:
    free(arr);

    return NULL;
}

Then you can do this:

int16_t ** array2d = create_2d_array(5,6);
if(!array2d) { /* Handle error */ }