Do I cast the result of malloc?
TL;DR
int *sieve = (int *) malloc(sizeof(int) * length);
has two problems. The cast and that you're using the type instead of variable as argument for sizeof. Instead, do like this:
int *sieve = malloc(sizeof *sieve * length);
Long version
No; you don't cast the result, since:
- It is unnecessary, as
void *
is automatically and safely promoted to any other pointer type in this case. - It adds clutter to the code, casts are not very easy to read (especially if the pointer type is long).
- It makes you repeat yourself, which is generally bad.
- It can hide an error if you forgot to include
<stdlib.h>
. This can cause crashes (or, worse, not cause a crash until way later in some totally different part of the code). Consider what happens if pointers and integers are differently sized; then you're hiding a warning by casting and might lose bits of your returned address. Note: as of C99 implicit functions are gone from C, and this point is no longer relevant since there's no automatic assumption that undeclared functions returnint
.
As a clarification, note that I said "you don't cast", not "you don't need to cast". In my opinion, it's a failure to include the cast, even if you got it right. There are simply no benefits to doing it, but a bunch of potential risks, and including the cast indicates that you don't know about the risks.
Also note, as commentators point out, that the above talks about straight C, not C++. I very firmly believe in C and C++ as separate languages.
To add further, your code needlessly repeats the type information (int
) which can cause errors. It's better to de-reference the pointer being used to store the return value, to "lock" the two together:
int *sieve = malloc(length * sizeof *sieve);
This also moves the length
to the front for increased visibility, and drops the redundant parentheses with sizeof
; they are only needed when the argument is a type name. Many people seem to not know (or ignore) this, which makes their code more verbose. Remember: sizeof
is not a function! :)
While moving length
to the front may increase visibility in some rare cases, one should also pay attention that in the general case, it should be better to write the expression as:
int *sieve = malloc(sizeof *sieve * length);
Since keeping the sizeof
first, in this case, ensures multiplication is done with at least size_t
math.
Compare: malloc(sizeof *sieve * length * width)
vs. malloc(length * width * sizeof *sieve)
the second may overflow the length * width
when width
and length
are smaller types than size_t
.
In C, you don't need to cast the return value of malloc
. The pointer to void returned by malloc
is automagically converted to the correct type. However, if you want your code to compile with a C++ compiler, a cast is needed. A preferred alternative among the community is to use the following:
int *sieve = malloc(sizeof *sieve * length);
which additionally frees you from having to worry about changing the right-hand side of the expression if ever you change the type of sieve
.
Casts are bad, as people have pointed out. Especially pointer casts.
You do cast, because:
- It makes your code more portable between C and C++, and as SO experience shows, a great many programmers claim they are writing in C when they are really writing in C++ (or C plus local compiler extensions).
- Failing to do so can hide an error: note all the SO examples of confusing when to write
type *
versustype **
. - The idea that it keeps you from noticing you failed to
#include
an appropriate header file misses the forest for the trees. It's the same as saying "don't worry about the fact you failed to ask the compiler to complain about not seeing prototypes -- that pesky stdlib.h is the REAL important thing to remember!" - It forces an extra cognitive cross-check. It puts the (alleged) desired type right next to the arithmetic you're doing for the raw size of that variable. I bet you could do an SO study that shows that
malloc()
bugs are caught much faster when there's a cast. As with assertions, annotations that reveal intent decrease bugs. - Repeating yourself in a way that the machine can check is often a great idea. In fact, that's what an assertion is, and this use of cast is an assertion. Assertions are still the most general technique we have for getting code correct, since Turing came up with the idea so many years ago.
As others stated, it is not needed for C, but necessary for C++. If you think you are going to compile your C code with a C++ compiler, for whatever reasons, you can use a macro instead, like:
#ifdef __cplusplus
# define NEW(type, count) ((type *)calloc(count, sizeof(type)))
#else
# define NEW(type, count) (calloc(count, sizeof(type)))
#endif
That way you can still write it in a very compact way:
int *sieve = NEW(int, 1);
and it will compile for C and C++.
From the Wikipedia:
Advantages to casting
Including the cast may allow a C program or function to compile as C++.
The cast allows for pre-1989 versions of malloc that originally returned a char *.
Casting can help the developer identify inconsistencies in type sizing should the destination pointer type change, particularly if the pointer is declared far from the malloc() call (although modern compilers and static analyzers can warn on such behaviour without requiring the cast).
Disadvantages to casting
Under the ANSI C standard, the cast is redundant.
Adding the cast may mask failure to include the header stdlib.h, in which the prototype for malloc is found. In the absence of a prototype for malloc, the standard requires that the C compiler assume malloc returns an int. If there is no cast, a warning is issued when this integer is assigned to the pointer; however, with the cast, this warning is not produced, hiding a bug. On certain architectures and data models (such as LP64 on 64-bit systems, where long and pointers are 64-bit and int is 32-bit), this error can actually result in undefined behaviour, as the implicitly declared malloc returns a 32-bit value whereas the actually defined function returns a 64-bit value. Depending on calling conventions and memory layout, this may result in stack smashing. This issue is less likely to go unnoticed in modern compilers, as they uniformly produce warnings that an undeclared function has been used, so a warning will still appear. For example, GCC's default behaviour is to show a warning that reads "incompatible implicit declaration of built-in function" regardless of whether the cast is present or not.
If the type of the pointer is changed at its declaration, one may also, need to change all lines where malloc is called and cast.
Although malloc without casting is preferred method and most experienced programmers choose it, you should use whichever you like having aware of the issues.
i.e: If you need to compile C program as C++ (Although it is a separate language) you must cast the result of use malloc
.