Take the 2-minute tour ×

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);  
    dates[i] =  malloc(sizeof(char)*16);
    if (dates[i] == NULL) {

        return NULL;
share|improve this question
To make the code more readable, you should use sizeof(DateTime *)in the first call to malloc. Or even better, since they are strings, don't create the typedef at all, but go with plain char * everywhere. –  Joachim Pileborg Aug 9 '12 at 13:02
Also, the two big things that's wrong that I can see is that you loop one to many, and you allocate one to little for the actual date string. If there's anything else that wrong you have to be more specific than just asking "whats wrong". –  Joachim Pileborg Aug 9 '12 at 13:05
Writing malloc(sizeof(char*)) instead of malloc(sizeof(*dates)) is not a good idea. Casting the return of malloc is wrong too. stackoverflow.com/questions/605845/… –  Eregrith Aug 9 '12 at 13:23

4 Answers 4


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

share|improve this answer

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);
share|improve this answer

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

share|improve this answer

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


is looping once to many.

Another problem is this:


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.

share|improve this answer

Your Answer


