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>

Solution 1

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;
}

Solution 2

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.

Comments

[Basile Starynkevitch]

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)

[Vality]

@BasileStarynkevitch Thanks, had forgotten all about abort, changed the example to use it now.

[chux - Reinstate Monica]

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]

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).

[Vality]

@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.

[Kevin]

@Vality: That's a bit simplistic. The POSIX standard contains the same language, 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.

[Ayxan Haqverdili]

Basically reimplementing xmalloc

[Vality]

@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.