Velocity Reviews - Computer Hardware Reviews

Velocity Reviews > Newsgroups > Programming > C Programming > why is casting malloc a bad thing?

Reply
Thread Tools

why is casting malloc a bad thing?

 
 
Brian Blais
Guest
Posts: n/a
 
      01-23-2004
Hello,

I saw on a couple of recent posts people saying that casting the return
value of malloc is bad, like:

d=(double *) malloc(50*sizeof(double));

why is this bad? I had always thought (perhaps mistakenly) that the
purpose of a void pointer was to cast into a legitimate date type. Is
this wrong? Why, and what is considered to be correct form?

thanks,

Brian Blais

 
Reply With Quote
 
 
 
 
Roberto DIVIA
Guest
Posts: n/a
 
      01-23-2004
Brian Blais wrote:
> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:
>
> d=(double *) malloc(50*sizeof(double));
>
> why is this bad? I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type. Is
> this wrong? Why, and what is considered to be correct form?


AFAIK it is wrong as <stdlib.h> should already have declared malloc the right
way. By overriding the standard declaration, you risk to re-declare malloc() in
a way it is not ment to. In other words, the <stdlib.h> declaration is the only
correct one, it must work and if it does not it means either that it's broken
or that <stdlib.h> has not been included.

A nice description of the "problem" can be found in:
http://users.powernet.co.uk/eton/c/hackerhotel.html

Ciao,
--
Roberto Divia` Love at first sight is one of the greatest
============= labour-saving devices the world has ever seen.
Mailbox: C02110 CERN-European Organization for Nuclear Research
E-mail: http://www.velocityreviews.com/forums/(E-Mail Removed) CH-1211 GENEVE 23, Switzerland
 
Reply With Quote
 
 
 
 
Joona I Palaste
Guest
Posts: n/a
 
      01-23-2004
Brian Blais <(E-Mail Removed)> scribbled the following:
> Hello,


> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:


> d=(double *) malloc(50*sizeof(double));


> why is this bad? I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type. Is
> this wrong? Why, and what is considered to be correct form?


It's not exactly _wrong_ but doesn't help anything either. Why people
discourage it is because of these reasons:
1) There is no need to. malloc(), correctly prototyped, returns a
void *, which is assignable to any object pointer type anyway.
2) If malloc() is not correctly prototyped, casting the return value
won't fix anything. malloc() will still return int, and casting that
int to an object pointer type isn't magically going to change it to a
pointer.
3) If malloc() is not correctly prototyped, the code is broken, because
it causes undefined behaviour. However casting the malloc() will make
the compiler _think_ it's not broken, although it really is.
The correct form is simply:
d = malloc(50*sizeof(double));
or:
d = malloc(50*sizeof *d);

--
/-- Joona Palaste ((E-Mail Removed)) ------------- Finland --------\
\-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
"Hasta la Vista, Abie!"
- Bart Simpson
 
Reply With Quote
 
Mark A. Odell
Guest
Posts: n/a
 
      01-23-2004
Brian Blais <(E-Mail Removed)> wrote in news:(E-Mail Removed):

> Hello,
>
> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:
>
> d=(double *) malloc(50*sizeof(double));


See the C-FAQ as to why.

http://www.eskimo.com/~scs/C-faq/q7.6.html
http://www.eskimo.com/~scs/C-faq/q7.7.html

> why is this bad? I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type.


Negative. Void pointers can hold any type of pointer so they naturally
"become" the desired type.

E.g.

void foo(void *pThing)
{
int idx;
char *pChar = pThing;

/* work with pChar */
for (idx = 0; idx < 10; ++idx)
{
pChar[idx] = 'a';
}
}

void bar(void)
{
int arr[100];

foo(arr);
}

No icky casts needed or desired. Void is your friend.

--
- Mark ->
--
 
Reply With Quote
 
Artie Gold
Guest
Posts: n/a
 
      01-23-2004
Brian Blais wrote:
> Hello,
>
> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:
>
> d=(double *) malloc(50*sizeof(double));
>
> why is this bad? I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type. Is
> this wrong? Why, and what is considered to be correct form?
>


A pointer to void (void *) can be *implicitly* converted from and to a
pointer to any object type, making the cast unnecessary. It is likely
that the confusion arises from the fact that this was not the case in
pre-ANSI (referred to as K&R) C, where the explicit cast *was* necessary.

The preferred for of the call to malloc() above is:

d = malloc(50 * sizeof *d);

This has several advantages:

It's shorter.

If one fails to #include <stdlib.h> (in which case the return type of
malloc() reverts to implicit `int') an error will be generated. This is
particularly significant on platforms where the size of `int' and `void
*' differ; in such cases the explicit cast would allow the code to
compile, but behave strangely..

If the type of `d' changes, the form of the malloc() call does not have
to be changed.

In general, casting is evil, except where it's necessary. Since it isn't
necessary in this case, why do it?


This issue has appeared repeatedly on news:comp.lang.c. If you have
further questions, I would recommend searching the archives (through,
for example, http://groups.google.com).

HTH,
--ag
--
Artie Gold -- Austin, Texas
 
Reply With Quote
 
Richard Bos
Guest
Posts: n/a
 
      01-23-2004
Brian Blais <(E-Mail Removed)> wrote:

> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:
>
> d=(double *) malloc(50*sizeof(double));
>
> why is this bad?


For the same reason that hanging a "danger - high voltage" sign on a
normal battery is wrong.
Casts are used to circumvent the C type system. If you use them where
the type system does not need to be circumvented, you give off the wrong
signals: that something odd is going on on that line (which it isn't),
and that you're unsure about how the type system works (which you
shouldn't be).

> I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type. Is
> this wrong?


This is quite the opposite of right. The purpose of a void * is to help
you _not_ need useless casts all over the place.

Richard
 
Reply With Quote
 
xarax
Guest
Posts: n/a
 
      01-23-2004
"Brian Blais" <(E-Mail Removed)> wrote in message
news:(E-Mail Removed)...
> Hello,
>
> I saw on a couple of recent posts people saying that casting the return
> value of malloc is bad, like:
>
> d=(double *) malloc(50*sizeof(double));
>
> why is this bad? I had always thought (perhaps mistakenly) that the
> purpose of a void pointer was to cast into a legitimate date type. Is
> this wrong? Why, and what is considered to be correct form?
>
> thanks,


You will find folks on both sides of the issue.
The main (some would say *only*) argument against
casting the result is that it will hide the failure
to include <stdlib.h> which declares malloc to return
(void *). The compiler will assume it returns (int),
and casting (int) to (something *) will cause undefined
behavior.

The folks on the other side of the argument say that
their compiler will complain loudly when a function
is referenced without a declaration (implementation-defined behavior), so the
question about including <stdlib.h>
is moot. Once past the question of proper malloc()
declaration, one must contend with the whether the
target pointer is what one intended.

======================
#include <stdlib.h>

static void fubar(void)
{
float * d; /* programmer changed this...! */

/* some dozen lines later in the code */

/* silent erroneous size calculation */
d = malloc(50*sizeof(double));
}
======================

If the programmer decides to change the declaration
of "d", the compiler will *NOT* catch the bad assignment.

If the assignment used a cast (double *), then the
compiler would complain about incompatible pointer
types.

======================
#include <stdlib.h>

static void fubar(void)
{
float * d; /* programmer changed this...! */

/* some dozen lines later in the code */

/* incompatible pointer assignment */
d = (double *) malloc(50*sizeof(double));
}
======================



Another suggestion is *not* to use "sizeof(double)",
but use "sizeof *d", so that the element size changes
automatically when changing the target declaration.

======================
#include <stdlib.h>

static void fubar(void)
{
struct gonk * d; /* programmer changed this...! */

/* some dozen lines later in the code */

/* 50 elements of whatever "d" points to... */
d = malloc(50*sizeof *d);
}
======================

Now the malloc() is compatible with changing the
target type of the pointer variable "d". You can
change it to any kind of pointer type and the
malloc will be compatible. This is the recommended
way to use malloc(), so that:

1. A missing <stdlib.h> is caught.

2. The malloc() assignment is always compatible
with any pointer type.

3. The size calculation changes along with any
change to the target declaration.


If you insist on using sizeof(double) in the size
calculation, then you should cast the result so that
the compiler will complain when the target type is
changed (so that you can change the sizeof() and
recompile). However, you will save yourself some
maintenance headaches by using: "d = malloc(50*sizeof *d);"



2 cents worth. Your mileage may vary.

--
----------------------------
Jeffrey D. Smith
Farsight Systems Corporation
24 BURLINGTON DRIVE
LONGMONT, CO 80501-6906
http://www.farsight-systems.com
z/Debug debugs your Systems/C programs running on IBM z/OS!
Are ISV upgrade fees too high? Check our custom product development!


 
Reply With Quote
 
Joona I Palaste
Guest
Posts: n/a
 
      01-23-2004
xarax <(E-Mail Removed)> scribbled the following:
> "Brian Blais" <(E-Mail Removed)> wrote in message
> news:(E-Mail Removed)...
>> Hello,
>>
>> I saw on a couple of recent posts people saying that casting the return
>> value of malloc is bad, like:
>>
>> d=(double *) malloc(50*sizeof(double));
>>
>> why is this bad? I had always thought (perhaps mistakenly) that the
>> purpose of a void pointer was to cast into a legitimate date type. Is
>> this wrong? Why, and what is considered to be correct form?
>>
>> thanks,


> You will find folks on both sides of the issue.
> The main (some would say *only*) argument against
> casting the result is that it will hide the failure
> to include <stdlib.h> which declares malloc to return
> (void *). The compiler will assume it returns (int),
> and casting (int) to (something *) will cause undefined
> behavior.


> The folks on the other side of the argument say that
> their compiler will complain loudly when a function
> is referenced without a declaration (implementation-defined behavior), so the
> question about including <stdlib.h>
> is moot. Once past the question of proper malloc()
> declaration, one must contend with the whether the
> target pointer is what one intended.


I don't understand this argument at all. Without the correct
prototype, using the return value of malloc() causes undefined
behaviour, *CAST OR NO CAST*. The code is broken anyway. *With*
the correct prototype, the cast is completely needless.

> ======================
> #include <stdlib.h>


> static void fubar(void)
> {
> float * d; /* programmer changed this...! */


> /* some dozen lines later in the code */


> /* silent erroneous size calculation */
> d = malloc(50*sizeof(double));
> }
> ======================


> If the programmer decides to change the declaration
> of "d", the compiler will *NOT* catch the bad assignment.


This is why most folks prefer to write:
d = malloc(50*sizeof *d);
and let that solve the problem for them.

> If the assignment used a cast (double *), then the
> compiler would complain about incompatible pointer
> types.


Which would not have ever arisen if they had used the form I
presented above.

> ======================
> #include <stdlib.h>


> static void fubar(void)
> {
> float * d; /* programmer changed this...! */


> /* some dozen lines later in the code */


> /* incompatible pointer assignment */
> d = (double *) malloc(50*sizeof(double));
> }
> ======================


> Another suggestion is *not* to use "sizeof(double)",
> but use "sizeof *d", so that the element size changes
> automatically when changing the target declaration.


Yes, like I just described.

(Snip example)

--
/-- Joona Palaste ((E-Mail Removed)) ------------- Finland --------\
\-- http://www.helsinki.fi/~palaste --------------------- rules! --------/
"'It can be easily shown that' means 'I saw a proof of this once (which I didn't
understand) which I can no longer remember'."
- A maths teacher
 
Reply With Quote
 
Mark Bruno
Guest
Posts: n/a
 
      01-23-2004
Casting of void* is required in C++, so i usually do it in C also. It's not
required, but it makes it easier when switching between C and C++, like I
often do.


 
Reply With Quote
 
Mark A. Odell
Guest
Posts: n/a
 
      01-23-2004
"Mark Bruno" <(E-Mail Removed)> wrote in
news:(E-Mail Removed):

> Casting of void* is required in C++, so i usually do it in C also. It's
> not required, but it makes it easier when switching between C and C++,
> like I often do.


So are you saying if I didn't name all my loop variables 'new', 'delete',
'cout', 'cin', etc. I'd have an easier time porting? BTW, why would you
ever use malloc() in C++?

--
- Mark ->
--
 
Reply With Quote
 
 
 
Reply

Thread Tools

Posting Rules
You may not post new threads
You may not post replies
You may not post attachments
You may not edit your posts

BB code is On
Smilies are On
[IMG] code is On
HTML code is Off
Trackbacks are On
Pingbacks are On
Refbacks are Off


Similar Threads
Thread Thread Starter Forum Replies Last Post
Bad media, bad files or bad Nero? John Computer Information 23 01-08-2008 09:17 PM
findcontrol("PlaceHolderPrice") why why why why why why why why why why why Mr. SweatyFinger ASP .Net 2 12-02-2006 03:46 PM
ActiveX apologetic Larry Seltzer... "Sun paid for malicious ActiveX code, and Firefox is bad, bad bad baad. please use ActiveX, it's secure and nice!" (ok, the last part is irony on my part) fernando.cassia@gmail.com Java 0 04-16-2005 10:05 PM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 12 02-23-2005 03:28 AM
24 Season 3 Bad Bad Bad (Spoiler) nospam@nospam.com DVD Video 0 02-19-2005 01:10 AM



Advertisments