Another dynamic memory allocation bug.

Question

I'm trying to allocate memory for a multidimensional array (8 rows, 3 columns).

Here's the code for the allocation (I'm sure the error is clear for you)

char **ptr = (char **) malloc( sizeof(char) * 8);

for (i = 0; i < 3; i++)
    ptr[i] = (char *) malloc( sizeof(char) * 3);

The crash happens when I reference this:

ptr[3][0];

Unhandled exception at 0x0135144d in xxxx.exe: 0xC0000005: Access violation writing location 0xabababab.

Are there any recommended references/readings for this kind of subject?

Thanks.

Answer 1

I don't know offhand any books on memory allocation, but any introductory C tutorial should explain it. As for your bug, your for loop only initializes the first 3 rows, instead of the 8 you allocated. It should be:

for (i = 0; i < 8; i++)

Answer 2

char **ptr = (char **) malloc( sizeof(char *) * 8);

Before, you were allocating space for 8 chars. You want space for 8 pointers to char.

Michael pointed out the other error, writing the first char from a string you never allocated.

It may help if you use constants, like:

const int STRING_COUNT = 8;
const int STRING_LEN = 2;

char **ptr = (char **) malloc( sizeof(char *) * STRING_COUNT);

Answer 3

The first malloc() is wrong. It should be:

malloc(sizeof(char*) * 8)

A char* is 4 byte (or 8 byte... see P.S) whereas char is 1 byte. When you write ptr[3] the compiler will assume that you want to access to the base address of ptr + 3*sizeof(char*). So you will access to memory that you didn't allocate.

P.S:

To be more precise, char* is 4 byte on 32 bit systems and 8 byte on 64 bit systems.

Answer 4

There are two problems.

First, as others have pointed out, your initial malloc should be

malloc(sizeof(char *) * 8)

Second, you are initializing the first three elements in the 8-element array, but ptr[3][0] refers to the fourth element. Thus the crash.

Answer 5

ptr is an array of 8 pointers, not chars, so the first line should be:

char **ptr = (char **) malloc( sizeof(char*) * 8)

Because there are 8 pointers, the loop should go from 0 to 7:

for (i = 0; i < 8; i++)

You can also consider using a less prone to errors version of the first line:

char **ptr = (char **) malloc( sizeof(*ptr) * 8)

And the last one:

ptr[i] = (char *) malloc( sizeof(*ptr[i]) * 3);

The rule is: always take sizeof of dereferenced lvalue. If lvalue is ptr, then you need sizeof(*ptr). ptr[i] becomes sizeof(*ptr[i]).

Answer 6

There are several errors/inconsistencies in your code, but the first major one is the wrong sizeof in the first allocation. The error that you made could have easily been avoided if you followed some good-practice guidelines:

(1) As much as possible avoid using type names in statements. Type names belong in declarations, not in statements.

(2) Don't cast the result of memory allocation functions.

The first allocation should have looked as follows

char **ptr = malloc( 8 * sizeof *ptr );

Try to remember this as a generic pattern: malloc requestes should normally look as follows

pointer = malloc( count * sizeof *pointer );

Note: no type names are mentioned in the above statement.

Of course, you should also make up your mind about the first size of your 2D array. You attempt to allocate 8, then you only initialize 3. Why?