tags:

views:

288

answers:

5

I've been using it for years, i.e.:

text = (char *)malloc( sizeof(char[1234]) );

instead of:

text = (char *)malloc( sizeof(char) * 1234 );

People have told me it's dangerous, but none could say why. I checked the C spec and it's legal. Are there any pitfalls here?

+7  A: 

It may be legal, but there are a few pitfalls.

First, if you (or a later maintainer) blindly replace the type with an expression you'll have problems:

sizeof(char *) * 4 => sizeof(x) * 4  // OK
sizeof(char *[4])  => sizeof(x[4])   // NOT OK

Second, not all compilers may support variable length arrays:

sizeof(char *) * n  // ALWAYS SUPPORTED
sizeof(char *[n])   // MAY NOT BE SUPPORTED

Finally, it's not a very common idiom, so people reading the code might be momentarily confused.

Whether these pitfalls make it worth the effort to change your habits is up to you, of course :)

bdonlan
+3  A: 

It is legal, and it is safe to do it the way you said:

text = (char *)malloc( sizeof(char[1234]) );

Typically though people work with sizeof with the smallest most primitive data types though to get past including padding in the count.

You probably heard it was not safe because someone was thinking that the array could have padding in between elements.

But there will never be padding in between elements of an array. The C99 standard states that an array must be a contiguous part of memory.

You can have padding before or after an array though within a struct in C.

Brian R. Bondy
+3  A: 

I'd advise using variable instead of the type:

text = malloc(sizeof(*text)*1234));

This way when you realize that it would be good to support i18n, and change the definition from:

char *text;

to:

wchar_t *text;

you still get enough room for 1234 elements, without modifying all the allocations to switch them from 'char' to 'wchar_t' to match. Of course, the same applies to other types -- short vs. int vs. long, float vs. double, etc.

Jerry Coffin
+4  A: 

The idiom I use is:

someptr = malloc(number_elems * sizeof *someptr);

To me, this has the advantage that I don't need to worry about the malloc call if I need to change the type of the elements.

int *data;
data = malloc(100 * sizeof *data);

... later I realize data should be unsigned longs ...

unsigned long *data;
data = malloc(100 * sizeof *data); /* no change from previous version */
pmg
I agree with you (and Jerry Coffin) in theory, but that's a habit I never really got into.
Chris Lutz
+1 for proper use of parenthesis with this, the One True sizeof Usage.
unwind
A: 

There is a practical difference between the two. In the first version the number of objects needs to be a compile-time constant. In the second version the number of elements can be determined at run-time.

So, while both of

size_t x = sizeof(double[99]);
size_t y = sizeof(double)*99;

work as expected, the following example won't

void fun(int i) {
    size_t x = sizeof(double[i]);  // error, no compile-time constant
    size_t y = sizeof(double) * i; // Fine
}
sellibitze