Jenny B
Jenny B

Reputation: 209

Dynamic allocation of char** in C

I have a date type defined as typedef char* DateTime; the format is "dd/mm/yyyy-hh:mm" e.g. "08/08/2012-12:00"

and I want to allocate n string that are "dates". What is wrong with the following?

    DateTime*  dates = (DateTime* ) malloc(sizeof(char*) * n);  
for (int i = 0; i <= n; i++) {
    dates[i] =  malloc(sizeof(char)*16);
    if (dates[i] == NULL) {
        free(dates);

        return NULL;
    }
}

Upvotes: 0

Views: 343

Answers (4)

Some programmer dude
Some programmer dude

Reputation: 409136

As been pointed out be both me in a comment, and by others, the loop

for (int i = 0; i <= n; i++) {

is looping once to many.

Another problem is this:

dates[i] =  malloc(sizeof(char)*16);

The actual string is 16 characters, but since strings in C needs an extra terminator character ('\0') you need to allocate one character more, which means you should multiply by 17.

And as noted by Dan F you also have a potential memory leak.

The two biggest problems is the looping and allocation, as that will cause so called undefined behavior when overwriting unallocated memory.

Upvotes: 0

Happy Green Kid Naps
Happy Green Kid Naps

Reputation: 1673

Besides responses by @Dan and @cnicutar (both of which are spot on), note that the string literal "08/08/2012-12:00" contains 17 characters (not 16). While it's string length is 16, it contains the 16 characters that you see PLUS the '\0' character at the end that serves as the terminator. Also, sizeof(char) is one by definition. Finally, the idiomatic way to allocate memory using malloc would be -

DateTime *dates = malloc(n * sizeof *dates);

Upvotes: 3

Dan F
Dan F

Reputation: 17732

In addition to cnicutar's answer there is also this:

In the event that this condition is true:

if ( dates[i] == NULL )

you are only calling free on the overall array, and not freeing the elements before i in the array. This can lead to a significant memory leak

Upvotes: 2

cnicutar
cnicutar

Reputation: 182619

for (int i = 0; i <= n; i++) {
                   ^

In C arrays start from 0 so dates[n] cannot be accessed. Drop the =.

Upvotes: 7

Related Questions