Should I check if malloc() was successful?

Question

Should one check after each malloc() if it was successful? Is it at all possible that a malloc() fails? What happens then?

At school we were told that we should check, i.e.:

arr = (int) malloc(sizeof(int)*x*y);
if(arr==NULL){
    printf("Error. Allocation was unsuccessful. \n");
    return 1;
}

What is the practice regarding this? Can I do it this way:

if(!(arr = (int) malloc(sizeof(int)*x*y))
    <error>

In theory, yes. In reality, if malloc fails the operating system is probably about to crash. PS: Your second example is much harder to read than the first and should be rejected by a code review. — Steve Wellens, Nov 09 '14 at 18:54
`arr = (int) malloc(...)` is wrong, `malloc` returns a pointer. Apart from that: yes, you should check if it fails because it can fail. [Also, casting its return value is harmful.](https://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858) — The Paramagnetic Croissant, Nov 09 '14 at 18:54
@SteveWellens I would say that it is yes in theory and in practice, especially when big buffers are allocated. — AlexD, Nov 09 '14 at 18:57
@AlexD - True. But if you are allocating a buffer so large that malloc may fail, then I would say a re-design is in order. — Steve Wellens, Nov 09 '14 at 18:59
@SteveWellens Usually yes, I do agree. But one can come up with an application which uses "as much memory as possible". But my main reason to comment was that someone could misread your remark as "_in practice, do not check_". (It is another issue if one needs to add `if` to every `malloc` or just wraps it and terminate the application with a proper message.) — AlexD, Nov 09 '14 at 19:09
@AlexD - "in practice, do not check" I would say, "in practice, **sometimes** it's fine not to check since it clutters up the code space and if allocating 10 bytes fails, your program has already crashed or is about to". Note: I write code to be as fault tolerant as possible: I look both ways when crossing a one way street. — Steve Wellens, Nov 10 '14 at 04:07
This question code has nothing to do with checking `malloc` or not as asked. Both code snippets check whether `malloc` is null. The difference is [whether to use an assignment inside a conditional or not](https://stackoverflow.com/questions/151850/why-would-you-use-an-assignment-in-a-condition). — ggorlen, Dec 19 '20 at 16:18
Does this answer your question? [Why would you use an assignment in a condition?](https://stackoverflow.com/questions/151850/why-would-you-use-an-assignment-in-a-condition) — ggorlen, Dec 19 '20 at 16:19

Vality · Accepted Answer · 2014-11-15T22:01:18.087

27

This mainly only adds to the existing answer but I understand where you are coming from, if you do a lot of memory allocation your code ends up looking very ugly with all the error checks for malloc.

Personally I often get around this using a small malloc wrapper which will never fail. Unless your software is a resilient, safety critical system you cannot meaningfully work around malloc failing anyway so I would suggest something like this:

static inline void *MallocOrDie(size_t MemSize)
{
    void *AllocMem = malloc(MemSize);
    /* Some implementations return null on a 0 length alloc,
     * we may as well allow this as it increases compatibility
     * with very few side effects */
    if(!AllocMem && MemSize)
    {
        printf("Could not allocate memory!");
        exit(-1);
    }
    return AllocMem;
}

Which will at least ensure you get an error message and clean crash, and avoids all the bulk of the error checking code.

For a more generic solution for functions that can fail I also tend to implement a simple macrosuch as this:

#define PrintDie(...) \
    do \
    { \
    fprintf(stderr, __VA_ARGS__); \
    abort(); \
    } while(0)

Which then allows you to run a function as:

if(-1 == foo()) PrintDie("Oh no");

Which gives you a one liner, again avoiding the bulk while enabling proper checks.

edited Nov 15 '14 at 22:01

answered Nov 10 '14 at 13:13

Vality

6,577
3
27
48

3

Your `PrintDie` should call `abort`, not `exit`. Because it would be simpler to debug (on Linux, you'll even get a `core` dump, which you could analyze post-mortem with `gdb`) – Basile Starynkevitch Nov 10 '14 at 13:17
@BasileStarynkevitch Thanks, had forgotten all about abort, changed the example to use it now. – Vality Nov 10 '14 at 13:45
4

`if(NULL == AllocMem)` is the wrong test. With `MemSize == 0`, receiving a `malloc()` return value of `NULL` _is_ compliant behavior and not an allocation failure. Changing to `if(NULL == AllocMem && MemSize != 0)` fixes that. – chux - Reinstate Monica Nov 15 '14 at 13:03
@chux You are correct there that that is desirable behaviour for most implementations, it is a little tricky in the standard as it says (in C99 and C11): "If the size of the space requested is zero, the behavior is implementation-defined" (7.22.3 P1, ISO C11). However what you suggest is a common implementation in many compilers so I shall add a check to the code with a note. Thanks. – Vality Nov 15 '14 at 21:58
@Vality: That's a bit simplistic. The POSIX standard [contains the same language](http://pubs.opengroup.org/onlinepubs/9699919799/functions/malloc.html), but then goes on to specify that the return value is *either* `NULL` or some non-null pointer which you can `free(3)` but not dereference. POSIX also conforms to the C standard in every respect except that it specifies setting `errno` on failure, which looks quite useless to me since `ENOMEM` is the only legal error code, but I suppose you could do `errno = 0; ptr = malloc(...); if(errno) abort()`. Still looks like a cheap party trick to me. – Kevin Mar 13 '18 at 01:39
Basically reimplementing [xmalloc](https://www.freebsd.org/cgi/man.cgi?query=xmalloc&apropos=0) – Aykhan Hagverdili May 20 '20 at 10:47
1

@Ayxan it looks like it. Though I haven't seen xmalloc before as I've never developed on BSD. Looks like a useful function though. Thabks for the info. – Vality May 20 '20 at 19:39

score 13 · Answer 2 · edited May 21 '21 at 12:28

13

No need to cast malloc(). Yes, however, it is required to check whether the malloc() was successful or not. Let's say malloc() failed and you are trying to access the pointer thinking memory is allocated will lead to crash, so it it better to catch the memory allocating failure before accessing the pointer.

int *arr = malloc(sizeof(*arr));
if(arr == NULL)
{
printf("Memory allocation failed");
return;
}

edited May 21 '21 at 12:28

Guy Avraham

3,482
3
38
50

answered Nov 09 '14 at 18:56

Gopi

19,784
4
24
36

11

On a pedantic note, this answer promotes "check whether the malloc() was successful or not" - which is a good idea. But then it shows how to check the result of `malloc(sizeof(int))` and not OP's `malloc(sizeof(int)*x*y)`. Testing against `NULL` is sufficient for this `sizeof(int)`, but wrong for OP's `sizeof(int)*x*y`. Should `x*y` --> 0, a `NULL` return is compliant code and does not indicate a memory allocation. Better to use `if(arr == NULL && x != 0 && y != 0)`. – chux - Reinstate Monica Nov 15 '14 at 14:08

Should I check if malloc() was successful?

2 Answers2

Linked