10

I have seen a few different ways of doing malloc error checking. Is one way better than the other? Are some exit codes better than others? Is using fprintf with stderr better than using a printf statement? Is using a return instead of an exit better?

    ptr=(int*)malloc(n*sizeof(int));  //memory allocated using malloc
    if(ptr==NULL)                     
    {
        printf("Error! memory not allocated.");
        exit(0);
    }

    ptr=(int*)malloc(n*sizeof(int));  //memory allocated using malloc
    if(ptr==NULL)                     
    {
        printf("Error! memory not allocated.");
        exit(1);
    }

    res = malloc(strlen(str1) + strlen(str2) + 1);
    if (!res) {
        fprintf(stderr, "malloc() failed: insufficient memory!\n");
        return EXIT_FAILURE;
    }

    ptr=(int*)malloc(n*sizeof(int));  //memory allocated using malloc
    if(ptr==NULL)                     
    {
        printf("Error! memory not allocated.");
        exit(-1);
    }

   ptr=(int*)malloc(n*sizeof(int));  //memory allocated using malloc
    if(ptr==NULL)                     
    {
        printf("Error! memory not allocated.");
        exit(EXIT_FAILURE);
    }

char *ptr = (char *)malloc(sizeof(char) * some_int);
if (ptr == NULL) {
    fprintf(stderr, "failed to allocate memory.\n");
    return -1;
}

char* allocCharBuffer(size_t numberOfChars) 
{
    char *ptr = (char *)malloc(sizeof(char) * numberOfChars);
    if (ptr == NULL) {
        fprintf(stderr, "failed to allocate memory.\n");
        exit(-1);
    }
    return ptr;
}
rockstar797
  • 137
  • 1
  • 2
  • 11
  • Errors should be written to stderr. When exiting with an error, exit status should be non-zero. If a malloc failure should result in a fatal error and immediate exit, consider wrapping it in a function that reports the error and exits, so you don't have to check the return value in your code. – Tom Karzes Jan 27 '16 at 01:00
  • The book is out of print now, but "C Unleashed" by Richard Heathfield et al has an interesting chapter about strategies you can use to try to *recover* from `malloc` failures. A user of a word processor doesn't want the application to crash just because the operation that they are attempting runs into memory resource limits. At the very least, if there is some relevant notion of state, perhaps state can be saved somehow before exiting. Don't think that a `malloc` failure has to automatically terminate your program. – John Coleman Jan 27 '16 at 01:08
  • 1
    [Do not cast the return value of `malloc`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Jonathon Reinhart Jan 27 '16 at 01:20
  • 1
    This has nothing to do with malloc specifically, does it? Plenty of things can fail. – user253751 Jan 27 '16 at 01:37
  • @rockstar797 don't forget to add an answer as accepted if your problem is soved ;-) – Magix Jan 27 '16 at 02:16
  • @rockstar797 Haha that's a common problem :p you are supposed to accept the one which solved your problem but here, i understand it can be a bit confusing – Magix Jan 27 '16 at 03:23
  • @Magix not sure which one to accept. They are all good answers. Every answer seems to clarify it a bit more. Wish I could combine them all into one answer. – rockstar797 Jan 27 '16 at 03:24

5 Answers5

13

How about this wrapper:

void *safe_malloc(size_t n)
{
    void *p = malloc(n);
    if (p == NULL) {
        fprintf(stderr, "Fatal: failed to allocate %zu bytes.\n", n);
        abort();
    }
    return p;
}

Then just use safe_malloc everywhere, and don't worry about the error checking.

Many programs are not written to gracefully handle memory allocation failures, and this solution is just fine for those applications. If your application is able to continue after memory allocation failures, then you probably wouldn't be asking this question.

Jonathon Reinhart
  • 132,704
  • 33
  • 254
  • 328
  • Downvoter care to explain why you dislike this solution? – Jonathon Reinhart Jan 27 '16 at 01:17
  • I downvoted because, depending of the context, the application may need to `free` some variables before leaving, even in applications which don't need to continue after allocation failures. This is not possible with this wrapper, and may cause memory leaks. – Magix Jan 27 '16 at 01:25
  • 1
    I didn't downvote, but aborting the program after an allocation failure is hardly "safe" when you have other memory that needs freed... That aside, you also might want to change `%zd` to `%zu` in your `printf()` format string to match `size_t` rather than the signed equivalent (`ssize_t` on a POSIX system). –  Jan 27 '16 at 01:26
  • 2
    @Magix Memory leaks are the least of your worries when your application is out of memory. All of the process's memory is returned to the kernel when it exits anyway. – Jonathon Reinhart Jan 27 '16 at 01:27
  • 1
    @Chrono thanks, changed to %zu. But again, freeing memory before you abort is pointless. – Jonathon Reinhart Jan 27 '16 at 01:29
  • Also if you are going to downvote this answer for that reason you need to downvote all other answers as well. – Jonathon Reinhart Jan 27 '16 at 01:30
  • @JonathonReinhart would this function be called like this? `safe_malloc(sizeof(n))` Are we assuming this is a char? Wouldn't you need to something a bit different if its int because its bigger? – rockstar797 Jan 27 '16 at 05:23
  • @rockstar797 it obviously depends on what you're trying to allocate. As I said, this function is a drop-in replacement for malloc that requires no error checking. – Jonathon Reinhart Jan 27 '16 at 11:18
  • Please note that fprintf may also perform mallocs. So then that printf may also fail. – Folkert van Heusden Nov 19 '20 at 15:01
10

When you detect an error with malloc(), calloc() and realloc() (i.e they return a NULL pointer), the POSIX98 standard dictates that errno must be set (see man malloc). You can then use the standard function perror() to print the error without the need to do your own formatting. Note that it will automatically print to stderr, no need to bother with that.

Furthermore, If your application considers the error as fatal, then the process must be ended. If your code is located in the main() function, then using return EXIT_FAILURE; is fine, and use exit(EXIT_FAILURE); if not. It is not recommended to exit with your own return code in that case. If the error is not considered as fatal, then it's up to you how to handle it.

Please also note that, when realloc() fails and returns NULL, the old pointer is still valid and must therefore be freed before leaving.

Magix
  • 4,989
  • 7
  • 26
  • 50
  • 1
    I wouldn't go as far as to dictate that allocation failures should always be considered fatal. That is really up to the application. The application may have a way to recover such as freeing up some memory and retrying the allocation or retrying with a smaller allocation or even just waiting for a while and trying again later in cases where memory usage across the application may be transient. – kaylum Jan 27 '16 at 01:10
  • @M.M I didn't mean to check for errors using errno, but only if errors are detected then `perror()` will do its job. I will edit to make this clearer – Magix Jan 27 '16 at 01:11
  • Updated : The Posix98 standard actually enforces `errno` to be set, as described in my answer. See `man malloc` and the posix standard. Non-compliant implementations are undefined behaviour – Magix Nov 15 '20 at 15:32
2

It is not about how you check the error, It is what you do with the error. In all the cases you can see that the common piece of code used is

if (ptr == NULL) {....}

When you encounter the return value is NULL, what you do after that is your personal choice. Some devs even like to assert() the program.

in the meantime gcc sets the errno for you. So you can use that to get more details and use it as well.

In summary you can do whatever suits you most for your program.

Fawzan
  • 4,738
  • 8
  • 41
  • 85
1

Rule #1. Always check malloc's return value (and realloc's) for errors.

Rule #2. Unless you can recover gracefully, always print an error message to stderr, giving relevant information.

Rule #3. Always exit with a nonzero status of there is an error. (The exact nonzero value doesn't matter so much.)

A "wrapper" function is an excellent way of addressing all three rules at the same time, if your program can tolerate exiting precipitously (that is, without saving any in-memory data which might be vital).

There are some exceptions to Rule #2, but they still involve reporting the error somehow (just not necessarily with an immediate fprintf to stderr), so the basic rule stands.

Steve Summit
  • 45,437
  • 7
  • 70
  • 103
0
perror("more details"); 

will (attempt to) print the error (as per the errno) to stderr. You can use that.

static void die(const char* msg){ perror(msg); exit(1); }  

You could also save the errno and translate it to a BSD style sysexits.h code or just somehow linearly serialize it to the parent process (by exiting with the errno or a linear translation thereof).

Petr Skocik
  • 58,047
  • 6
  • 95
  • 142