Why it is bad idea to check result of malloc call with assert

The pointer returned by malloc needs to be checked before use. Using the assert macro to check the pointer would be wrong. In this article, we'll investigate why this is a bad programming tip.

When I say malloc, I mean not only this function, but also calloc, realloc, _aligned_malloc, _recalloc, strdup, and so on.

The malloc function returns a null pointer if it's unable to allocate a memory buffer of the specified size. So, it's better to check if a pointer is equal to NULL before dereferencing it.

Why this check is mandatory, I have covered in detail in the article: "Four reasons to check what the malloc function returned". If you have the slightest doubt that such a check is necessary, please read the article before continuing this one.

The standard check looks like this:

int *ptr = malloc(sizeof(int) * N);
if (!p)
{
  // Handling memory allocation error
}

I prefer explicit comparison, though. It protects the code from typos and makes it a bit more readable (and it's obvious that a pointer is being checked):

if (p == NULL)

The most important thing to do is to handle the memory allocation error. The error may appear as follows:

• a program terminates with a return code (if such behavior is allowed);
• an operation is rejected, but the program continues working to save the data;
• if the program controls a device, it shuts down the device (if such behavior is allowed);
• the library should return a failure status, and the program will decide what to do next;
• other options depending on the type of the program.

Let's not assume that we can skip the check, because the program will crash anyway if a null pointer is dereferenced. It may not crash. Null pointer dereference is undefined behavior, and its signs can be very surprising to a programmer. I've covered all of this in the article I mentioned earlier.

Now, let's discuss the following anti-pattern:

int *ptr = malloc(sizeof(int) * N);
assert(ptr);
memcpy(ptr, foo, sizeof(int) * N);

Some programmers use the assert macro (or its analogs, such as the ASSERT macro from the MFC library) to check the pointer.

Please note that if the NDEBUG macro is declared, the assert macro turns off (does nothing). An example of an implementation from assert.h:

#ifdef NDEBUG
#define assert(condition) ((void)0)
#else
#define assert(condition) /*implementation defined*/
#endif

In reality, this means that in the release version of the program, which should run as fast as possible, the NDEBUG macro is declared and assert is turned into "nothing". As a result, there's no protection against null pointers, and the consequences can be quite unpleasant.

In the debug version of the program, assert works as it should, but it makes no sense in real cases.

Usually, when developers test the debug version of a program, they use simple operation scenarios that don't require large memory allocations. Most likely, severe memory fragmentation won't occur. It's very unlikely that there will be a situation where memory allocation is impossible.

If such a thing really happens, assert indeed detects a null pointer. However, this is almost useless for the debug version of the program. The null pointer dereference shows up immediately anyway. There won't be any tricky undefined behavior issues as there are in optimized code.

So, this is what we have:

• the assert macro doesn't work where it's supposed to (in release builds);
• but it works where it's of little relevance (in debug builds).

What happens if you don't declare the NDEBUG macro for release versions?

This is a bad idea. In fact, a memory allocation error always causes an application to crash. Such behavior is unacceptable for many types of applications (especially for libraries). Many developers also use the same macro to implement additional checks throughout the code where it's necessary. So, missing NDEBUG can be considered a bug and then be fixed. Just don't do that.

Never be lazy to write proper crash checks, and users will be grateful. Thank you for reading.

Additional links:

1. There are tons of other ways to create erroneous code: 60 terrible tips for a C++ developer
2. Null Pointer Dereferencing Causes Undefined Behavior
3. Shall we check pointer for NULL before calling free function?

Comments

[Paul J. Lucas]

Your post is true, but too specific about malloc(). You shouldn't use assert() to check anything that must be checked. You really should discuss the use of assert() in general such as I did.

BTW, this post should be tagged #c not #cpp since C uses malloc() whereas C++ generally uses new.

[Vincent Milum Jr]

If you check the common implementations of "new", its still "malloc" under the hood inside of the "new" function. C didn't stop existing in C++, so I think this is a perfectly valid tag still :)

And plenty of us still use malloc extensively in the C++ world too!

[Paul J. Lucas]

What certain implementations might do is irrelevant. It's what the standard says. In a C++ program, yes, you can still use malloc(), but you may not delete what has been malloc'd nor may you free() what has been new'd. An implementation is free to use different memory arenas for each. Unless you're allocating memory in C++ code that is to be free'd by C code in the same program, there's no justification for using malloc() in a C++ program: new is almost always better.