tags:

views:

536

answers:

5

Now before people start marking this a dup, I've read all the following, none of which provide the answer I'm looking for:

  1. C FAQ: What's wrong with casting malloc's return value?
  2. SO: Should I explicitly cast malloc()’s return value?
  3. SO: Needless pointer-casts in C
  4. SO: Do I cast the result of malloc?

Both the C FAQ and many answers to the above questions cite a mysterious error that casting malloc's return value can hide, however none of them give a specific example of such an error in practice. Now pay attention that I said error, not warning.

Now given the following code:

#include <string.h>
#include <stdio.h>
// #include <stdlib.h>

int main(int argc, char** argv) {

    char * p = /*(char*)*/malloc(10);
    strcpy(p, "hello");
    printf("%s\n", p);

    return 0;
}

Compiling the above code with gcc 4.2, with and without the cast gives the same warnings, and the exec executes properly and provides the same results in both cases.

anon@anon:~/$ gcc -Wextra nostdlib_malloc.c -o nostdlib_malloc
nostdlib_malloc.c: In function ‘main’:
nostdlib_malloc.c:7: warning: incompatible implicit declaration of built-in function ‘malloc’
anon@anon:~/$ ./nostdlib_malloc 
hello

So can anyone give a specific code example of a compile or runtime error that could occur because of casting malloc's return value, or is this just an urban legend?

Edit I've come across two well written arguments regarding this issue:

  1. In Favor of Casting: CERT Advisory: Immediately cast the result of a memory allocation function call into a pointer to the allocated type
  2. Against Casting
+6  A: 

Non-prototyped functions are assumed to return int.

So you're casting an int to a pointer. If pointers are wider than ints on your platform, this is highly risky behavior.

Plus, of course, that some people consider warnings to be errors, i.e. code should compile without them.

Personally, I think the fact that you don't need to cast void * to another pointer type is a feature in C, and consider code that does to be broken.

unwind
I have this belief that the compiler knows more about the language than I do, so if it warns me about something, I pay attention.
Jurily
In many projects, C code is compiled as C++ where you do need to cast the `void*`.
laalto
nit: "_by default_, non-prototyped functions are assumed to return `int`." -- Do you mean it is possible to change the return type of non-prototyped functions?
pmg
@pmg: of course, you can provide your own definitions of functions. :)
Michael Foukarakis
@laalto - It is, but it shouldn't be. C is C, not C++, and should be compiled with a C compiler, not a C++ compiler. There's no excuse: GCC (one of the best C compilers out there) runs on nearly every platform imaginable (and generates highly optimized code, too). What reasons could you possibly have to compile C with a C++ compiler, other than laziness and loose standards?
Chris Lutz
EffoStaff Effo
@EffoStaff: Mixing in the same translation unit is a Bad Idea, in my opinion. What Chris said, basically.
unwind
@EffoStaff: Don't you know `extern "C"`?
Ferdinand Beyer
Example of code which you might want to compile as both C and C++: `#ifdef __cplusplus \nextern "C" { \n#endif static inline uint16_t swb(uint16_t a) {return ((a << 8) | ((a >> 8) } \n#ifdef __cplusplus\n } \n#endif`. Now, why you'd want to call malloc in a static inline function I really don't know, but headers which work in both are hardly unheard of.
Steve Jessop
+11  A: 

You won't get a compiler error, but a compiler warning. As the sources you cite say (especially the first one), you can get an unpredictable runtime error when using the cast without including stdlib.h.

So the error on your side is not the cast, but forgetting to include stdlib.h. Compilers may assume that malloc is a function returning int, therefore converting the void* pointer actually returned by malloc to int and then to your your pointer type due to the explicit cast. On some platforms, int and pointers may take up different numbers of bytes, so the type conversions may lead to data corruption.

Fortunately, modern compilers give warnings that point to your actual error. See the gcc output you supplied: It warns you that the implicit declaration (int malloc(int)) is incompatible to the built-in malloc. So gcc seems to know malloc even without stdlib.h.

Leaving out the cast to prevent this error is mostly the same reasoning as writing

if (0 == my_var)

instead of

if (my_var == 0)

since the latter could lead to a serious bug if one would confuse = and ==, whereas the first one would lead to a compile error. I personally prefer the latter style since it better reflects my intention and I don't tend to do this mistake.

The same is true for casting the value returned by malloc: I prefer being explicit in programming and I generally double-check to include the header files for all functions I use.

Ferdinand Beyer
It would seem that since the compiler warns about the incompatible implicit declaration, then this is a non-issue as long as you pay attention to your compiler warnings.
Robert S. Barnes
This is maybe a completely separate question, but from an implementation point of view it doesn't seem to make sense that the compiler would link to the correct object code definition of `malloc`, yet convert it's return value to int and then convert it to T*. How is it that the implicit declaration actually generates object code?
Robert S. Barnes
@Robert: yes, given certain assumptions about the compiler. When people are giving advice for how best to write C *in general*, they can't assume that the person receiving the advice is using a recent version of gcc.
Steve Jessop
Oh, and the answer to the second question is that the caller contains the code to pick up the return value (which it thinks is an int), and convert it to T*. The callee just writes the return value (as a void*) and returns. So depending on the calling convention: int returns and void* returns may or may not be in the "same place" (register or stack slot); int and void* may or may not be the same size; converting between the two may or may not be a no-op. So it might "just work", or the value could be corrupted (some bits lost, perhaps), or the caller could pick up completely the wrong value.
Steve Jessop
+3  A: 

If you do this when compiling in 64-bit mode, your returned pointer will be truncated to 32-bits.

EDIT: Sorry for being too brief. Here's an example code fragment for discussion purposes.

main()
{
   char * c = (char *)malloc(2) ;
   printf("%p", c) ;
}

Suppose that the returned heap pointer is something bigger than what is representable in an int, say 0xAB00000000.

If malloc is not prototyped to return a pointer, the int value returned will initially be in some register with all the significant bits set. Now the compiler say, "okay, how do I convert and int to a pointer". That's going to be either a sign extension or zero extension of the low order 32-bits that it has been told malloc "returns" by omitting the prototype. Since int is signed I think the conversion will be sign extension, which will in this case convert the value to zero. With a return value of 0xABF0000000 you'll get a non-zero pointer that will also cause some fun when you try to dereference it.

Peeter Joot
Could you explain in detail how this would occur?
Robert S. Barnes
Especially on ILP64?
MSalters
i think Peeter Joot was figuring out that "By default, non-prototyped functions are assumed to return int" w/o including stdlib.h, and sizeof(int) is 32 bits while sizeof(ptr) is 64.
EffoStaff Effo
A: 

A Reusable Software Rule:

In the case of writing an inline function in which used malloc(), in order to make it reusable for C++ code too, please do an explicit type casting (e.g. (char*)); otherwise compiler will complain.

EffoStaff Effo
hopefully, with the (recent) inclusion of link-time optimizations in gcc (see http://gcc.gnu.org/ml/gcc/2009-10/msg00060.html ), declaring inline-functions in header files will no longer be necessary
Christoph
you got bad ideas. do you aware of that what is portable and cross-platform among different compilers/versions/architectures? ok, you may not. then what does reusable mean?
EffoStaff Effo
+5  A: 

One of the good higher-level arguments against casting the result of 'malloc' is often left unmentioned, even though, in my opinion, it is more important than the well-known lower-level issues (like truncating the pointer when the declaration is missing).

A good programming practice is to write code that is as type-independent as possible. This means, in particular, that type names should be mentioned in the code as little as possible or best not mentioned at all. This applies to casts (avoid unnecessary casts), types as arguments of 'sizeof' (avoid using type names in 'sizeof') and, generally, all other references to type names.

Type names belong in declarations. As much as possible, type names should be restricted to declarations and only to declarations.

From this point of view, this bit of code is bad

int *p;
...
p = (int*) malloc(n * sizeof(int));

and this is much better

int *p;
...
p = malloc(n * sizeof *p);

not simply because it "doesn't cast the result of 'malloc'", but rather because it is type-independent.

AndreyT
Fwiw, I think this is more or less the same reason as this: http://stackoverflow.com/questions/953112/should-i-explicitly-cast-mallocs-return-value/954785#954785 but focused on the type-independence rather than the DIY. Of course the first follows from the second (or vice versa), so at least it's mentioned *sometimes*. :)
unwind