[Discuss] Casting the return value of malloc bad?

[Paul Nienaber]

Adam Parkin wrote:
> Andrew Resch wrote:
>>> Paul Nienaber wrote:
>>>> Do it this way:
>>>>
>>>> foo = malloc(n * sizeof(*foo));
> >>
>>> Good point, but if the line immediately before the malloc looked like:
>>>
>>> SomeType * foo = NULL;
>>>
>>> then wouldn't sizeof (*foo) choke (NULL pointer dereference)?
>>
>> OH sorry about the last email..  I should read before replying.
>> The sizeof(*foo) will return the size of SomeType, not the length of
>> data that it is pointing to.
>
> Exactly, and that's why Paul's tip is good (so long as foo is pointing
> to a valid instance of SomeType).
It doesn't matter if it's pointing to anything.  sizeof is
compile-time.  It's an operator, not a function, and the value of the
expression you feed it is irrelevant.
>
> I actually experienced this week in my labs as I was showing the class
> why casting the return value of malloc isn't so good.  I wrote on the
> board:
>
> /* alloc memory for array of x ints */
> int * p = (int *) malloc (sizeof (int) * x);   
>
> And then said, "well what if you decide that you want an array of
> longs instead of ints, but forget to change the cast":
>
> long * p = (int *) malloc (sizeof(int) * x);
This isn't a *problem* with casting, though.  Your compiler will catch
you on this one.  The problems are the cases where you don't have a
valid prototype for malloc() in scope, and similar.  Those introduce
some REALLY subtle bugs (when the representation of int and size_t are
not the same, as on my box, for example).
>
> But then of course a smart student pointed out that the code is borked
> anyways because I forgot to change the sizeof(int) to sizeof(long). 
> So then I changed it to
>
> long * p = (int *) malloc (sizeof(long) * x);
>
> But the odds you'll remember to change the sizeof, but not change the
> cast are pretty slim so at this point the exercise became a bit
> artificial and contrived. =8-p
>
> I think in the end the best approach is something like:
>
> typedef int ArrayDataType;
> ArrayDataType * p = malloc (sizeof(ArrayDataType) * numElements);
>
> Now if you decide on an array of longs instead of ints, you only need
> change the typedef line and the malloc line remains the same.
>
> Side rant: typedef is one of the biggest things I miss going from
> C/C++ to Java, as often I want to change the type of data contained in
> a container class, but in order to do so there's usually multiple
> places where that information appears.  Usually I end up creating an
> artificial "DataElement" class which is just a wrapper for the type to
> be contained which often feels like swatting a fly with a bazooka.
> =8-p  Generics help with this, but if you have multiple containers
> then you still have multiple points where the data type appears and is
> still a bit of a maintenance problem.
I'm going to *strongly* disagree with putting the type of the variable
in your code more than once.  Use foo = malloc(n * sizeof(*foo));,
seriously, and encourage other people to use it.  Then you *can't* screw
up changing foo's type.  And no, there's no dereference.  *foo is simply
an expression, which has a type, which has a size.  This is all
evaluated at compile time.  (interesting point:  int *foo = NULL;
printf("%p\n", &*foo); is valid -- no dereference occurs there, either =)

The deal with not writing in the type is that, in real (read: not the
crap you do in beginner CSC courses) typically involves assigning to a
(pointer) variable at some point OTHER than where it is declared, and
therefore the use of the variable to determine the size of the type it
points to is REALLY, REALLY valuable in making your code easier to maintain.

~p