On casting the return value of malloc() and company

TL;DR: Don't.

There's an ages old debate on whether or not you should cast the return value of malloc(), calloc(), and realloc(). My two cents are that you should not do this. Now let me convince you a little bit.

The single biggest reason not to cast the return values of these functions is to avoid bugs. This is not theoretical, I have literally seen things like this happen when teaching people C. What I've observed is people forgetting to #include <stdlib.h>, use malloc() correctly, and then getting a segfault when they try to run their code. The segfault goes away when they add the missing #include. Note that this doesn't happen 100% of the time, and I'm having problems creating a reproducible sample, or I'd post that.

Why would that happen? It's a series of things. First, by failing to #include <stdio.h>, there is not prototype for malloc(). Since there is no declaration, it winds up getting implicitly declared at the points of its first use. The problem with implicit declaration is that the return type of all implicitly declared functions is int. On platforms where sizeof(int) == sizeof(void *), this isn't much of a problem, and such bugs can go for years without notice. On platforms where this is not the case, the compiler will force the return value to fit into the range of int before casting that to a pointer, which winds up making the pointer point to the entirely wrong place.

Granted, compilers have gotten better at warning about this kind of thing recently, but the explicit cast prevents the compiler from warning you about assigning an int value to a pointer, which is the underlying cause of this bug.

The other reason to omit the cast is to improve readability. It's simply not necessary. C allows assigning values of type void * to any other pointer type without cast. The compiler already knows the type of the lvalue, the reader can readily find it, the cast just adds noise.

This is where some people pipe up and ask about C++. My counter is that you shouldn't be calling malloc() and friends in C++ code, you should be using new. It's a language keyword for a reason. Only new will ensure that your default constructor gets called and your newly allocated objects are in any sort of consistent state.

But what about code that is meant to be shared between C and C++? I've seen that argument as well, and it's patently absurd. The only portion of such code that should be shared at the compiler level is the headers. That is, the interface. If your interface contains calls to malloc(), stop what you're doing, go back to CS101, and start over. Your implementation, on the other hand, is presumably written in pure C (if it's meant to support use in C and C++), and as such, should be compiled with a C compiler, not a C++ compiler. It really is as simple as that: Compile C code with a C compiler, C++ code with a C++ compiler.

Bonus rant: The thing you use with the sizeof operator should, if at all possible, be the dereferenced variable, not its type (e.g. int *i = malloc(sizeof(*i)); instead of int *i = malloc(sizeof(int));). This reduces maintenance burden should the underlying type need to change, and will always provide the correct size.

Thus, the platonic ideal of dynamically allocating an array of things:

struct thing *p = calloc(how_many, sizeof(*p));
Copyright © 2020 Jakob Kaivo <jakob@kaivo.net>