why is casting malloc a bad thing?

B

Brian Blais

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
 
R

Roberto DIVIA

Brian said:
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,
 
J

Joona I Palaste

Brian Blais said:
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);
 
M

Mark A. Odell

Brian Blais said:
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.
 
A

Artie Gold

Brian said:
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 If you have
further questions, I would recommend searching the archives (through,
for example, http://groups.google.com).

HTH,
--ag
 
R

Richard Bos

Brian Blais said:
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
 
X

xarax

Brian Blais said:
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!
 
J

Joona I Palaste

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 ([email protected]) ------------- 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
 
M

Mark Bruno

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.
 
M

Mark A. Odell

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++?
 
D

Default User

Mark said:
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.


You shouldn't be using malloc() in C++ period. Writing code to move back
and forth like that is poor idea. C++ is its own language, and you
should be writing C++ code using the modern libraries. Don't write
cripple C code for C++ applications.

I work in both languages, it's not that hard.



Brian Rodenborn
 
J

j

Brian Blais said:
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 do not have to cast a pointer to void as it can be converted to or from
a pointer to any incomplete or object type.

The issue with casting from malloc is not that it is just superfluous
but that you might be suppressing a warning that might otherwise
be useful.
 
E

E. Robert Tisdale

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

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

Why is this bad?

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

Why and what is considered to be correct form?

This is just a style issue.

I always use the explicit cast
to prevent my C++ compiler from complaining about

warning: invalid conversion from `void*' to `double*'

It applies the implicit conversion anyway so it doesn't matter
except that more important warnings some times get lost
if too many warning messages such as this are issued by my compiler.

The important thing here is to adopt a style and stick to it.
 
M

Mark A. Odell

Do you really work for NASA yet spew such rubbish?
It isn't.

It is not proper C. To the OP, ignore E. Robert Tisdale's response and
read the C-FAQ instead.

It is.
This is just a style issue.

It is not.
I always use the explicit cast
to prevent my C++ compiler from complaining about

You *shouldn't* use malloc in C++!
The important thing here is to adopt a style and stick to it.

True but this is not a style issue.
 
E

E. Robert Tisdale

Mark said:
It is not proper C.

Nonsense!
An explicit cast on the value returned from malloc
is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
and there is *no* C compiler that will complain about it.

Who said anything about using malloc in C++?
I'm just using my C++ compiler to compile C code.
True but this is not a style issue.

If it isn't a style issue, then it is nothing at all.
 
M

Mark A. Odell

Nonsense!
An explicit cast on the value returned from malloc
is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
and there is *no* C compiler that will complain about it.

It's not illegal, just not proper. Casts are a punt, you are side-stepping
the C type enforcement. In some cases you must do this (cast) but malloc()
is definitely not one of them.
Who said anything about using malloc in C++?
I'm just using my C++ compiler to compile C code.

Why when you can tell it to switch ISO C mode? Usually, just giving the
file a .c extension is all that is required. Otherwise, a flag is usually
available to force the file to be treated as C not C++.
If it isn't a style issue, then it is nothing at all.

Oh, it's still a punt.
 
E

Eric Sosman

E. Robert Tisdale said:
Nonsense!
An explicit cast on the value returned from malloc
is *not* explicitly os implicitly prohibited by the ANSI/ISO C standards
and there is *no* C compiler that will complain about it.

"Not proper" is perhaps too strong. "Not useful" or
"not needed" or "not smart" might have been better.
Who said anything about using malloc in C++?
I'm just using my C++ compiler to compile C code.

It's not C code if it's being compiled as C++. Perhaps
you could clarify with a simple test:

#include <stdio.h>
typedef int private;
private main(private class, char **new) {
puts ("Hello, world!");
return 0;
}

If your implementation fails to compile and run this program,
the code is not being processed as C but as something else.
 
E

E. Robert Tisdale

Eric said:
Perhaps you could clarify with a simple test:

#include <stdio.h>
typedef int private;
private main(private class, char **new) {
puts ("Hello, world!");
return 0;
}

If your implementation fails to compile and run this program,
the code is not being processed as C but as something else.
> cat main.c
#include <stdio.h>

typedef int private;
private main(private class, char* new[]) {
puts("Hello, world!");
return 0;
}
> g++ -x c++ -Dprivate=Private -Dclass=Class -Dnew=New \
-Wall -ansi -pedantic -o main main.c
main.c:4: warning: return type for `main' changed to `int'
> ./main Hello, world!
> gcc -x c -Dprivate=Private -Dclass=Class -Dnew=New \
-Wall -std=c99 -pedantic -o main main.c
Hello, world!
 
E

Eric Sosman

E. Robert Tisdale said:
Eric said:
Perhaps you could clarify with a simple test:

#include <stdio.h>
typedef int private;
private main(private class, char **new) {
puts ("Hello, world!");
return 0;
}

If your implementation fails to compile and run this program,
the code is not being processed as C but as something else.
cat main.c
#include <stdio.h>

typedef int private;
private main(private class, char* new[]) {
puts("Hello, world!");
return 0;
}
g++ -x c++ -Dprivate=Private -Dclass=Class -Dnew=New \
-Wall -ansi -pedantic -o main main.c
main.c:4: warning: return type for `main' changed to `int'
./main Hello, world!
gcc -x c -Dprivate=Private -Dclass=Class -Dnew=New \
-Wall -std=c99 -pedantic -o main main.c
Hello, world!

Care to try it again without the -D switches?

Here's the point I'm trying to make: The meaning of a source
file is not contained within the file itself, but only arises
when the file is handed to a compiler or interpreter or whatever.
A C compiler treats the source handed to it as C code; hand that
same source to a C++ compiler and it's C++ code. Hand it to a
COBOL compiler and it's COBOL code.

I once saw an ingeniously constructed source file that
implemented "Hello, world!" in three languages at once: C (pre-
Standard), Fortran (flavor unremembered), and csh. And in natural
languages we have the example of "Mots D'Heures, Gousses, Rames,"
a text that is simultaneously stilted French and phonetic English.
Such exercises, although entertaining and perhaps awe-inspiring,
are not especially practical.

Ordinarily, it's difficult to prepare a source file that's
meaningful in two or more different languages, and even more
difficult to get the different languages to assign the source the
same meaning. But because C and C++ diverged fairly recently,
they have enough in common that it's still possible to write such
ambivalent code fragments. "Possible" doesn't mean "Recommended"
or even "Sane," though: The writing is noticeably cramped, even
if not as perversely contorted as the ForcshtranC tour de farce.
Some may find it amusing to write such hermaphroditic code, but
a recommendation for or against a particular coding practice
should arise from something more than entertainment potential.

Do not pretend that C and C++ are interchangeable, even in
subsetted forms. They are closely related but different languages,
and the attempt to write ambivalent code gives a product that is
neither good C nor good C++. Choose your language and use it to
the hilt; there is no reason to weaken your code with compromise.
 
E

E. Robert Tisdale

Eric said:
Here's the point I'm trying to make:

[snip]

The point is that nobody claimed that,
"Every C program is a C++ program".

I can't afford to write C code
that cannot also be compiled by a C++ compiler.
We can't afford to support two versions of libraries
one for C programs and another for C++ programs.
We just write code that will compile as either one.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,744
Messages
2,569,484
Members
44,903
Latest member
orderPeak8CBDGummies

Latest Threads

Top