Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

jrockway's comment should really be addressed -- this can cause some really nasty crashes.


Really? How would dereferencing a NULL pointer cause "some really nasty crashes"?

Yes, it will crash. That is a Good Thing(tm).

1) It is extremely unlikely that malloc() will ever return NULL (on a PC). This is due to virtual memory. If you manage to allocate more than 1.5GB of memory, then yes. Otherwise no.

2) Even if malloc() does return NULL, then NULL checking it is pointless because program operation cannot realistically continue. You need that memory. The best and cleanest solution is to crash. You will get a call trace leading to the exact point of failure (which you won't get if you do NULL checks + continue program execution). Also, as far as I know, it's almost impossible for a dereferenced NULL pointer to be a security vulnerability, unlike e.g. a buffer overrun.

In summary, just let it crash. There isn't any reason not to, unless you're developing a "third-party library" (e.g. you're a Lua developer, etc) and you want to leave the decision of whether to crash to the programmers using your code.


Are you kidding? Most real software does something like allocating some emergency memory at startup time, and then handles a failing malloc call by using that reserve pool to save work, perhaps do a GC, and print a decent error message before quitting.

Losing all of the user's work and printing "Segmentation fault (core dumped)" is never the right answer.


The point is, the crash you describe will only occur on 32-bit operating systems, and only after you've allocated more than 1.5GB of memory. For most projects, that is extremely unlikely.

Your solution would work. But the extra code complexity, development time, and maintenance isn't worth it except in very, very specific scenarios, like if you're writing a third-party library such as Lua or FMOD or FreeType or... etc.

Here's an alternative solution that doesn't require NULL checking malloc, and provides all of the same benefits:

1) create a function (I call mine "Sys_Exit()") which allows you to cleanly exit your application from anywhere in your codebase. For example in my game engine, my main() function looks like:

  int main()
  {
    App_Startup();
    while ( App_Frame() ) {}
    App_Shutdown();
    return 0;
  }
and Sys_Exit() looks like:

  void Sys_Exit( int code )
  {
    App_Shutdown();
    exit( code );
  }
2) write a "my_malloc" function which forwards to malloc. Use it for all memory allocation. When it detects that more than 1.2GB of memory has been allocated, then it prints an error message and calls Sys_Exit().

This allows you to save the user's work, etc, and your shutdown sequence has a fairly large amount of remaining memory to work with (so that you don't truly run out of memory after you've fake-run-out-of-memory).

This is a very simple solution if you really care about the extremely unlikely out-of-memory crash.

----

tl;dr: It is almost always a bad idea to NULL check malloc(). It tends to destroy readability, is error prone, and is hard to maintain, for very little practical benefit.


Even ignoring the possibility of out-of-memory crashes, null pointer bugs can lead to memory corruption and code execution and have been popular targets in recent years: http://www.google.com/search?sourceid=chrome&ie=UTF-8...

Edit: http://flashmypassion.blogspot.com/2008/04/this-new-vulnerab... is a copy of a blog post (which seems to be missing from the Matasano chargen; odd) about Mark Dowd's crazy Flash null-pointer vuln. The first comment from Dino Dai Zovi (another crazy good security researcher) is very relevant:

> Oh, the sweet sweet vindication. I remember an argument that I have had twice in the last several years about whether one should check the return value of malloc. I argued that it should always be done for two reasons: Reading address zero might crash, but not necessarily zero-plus-offset and because a compare register to zero is so free performance-wise that it isn’t even funny.

> Guess who was arguing the contrary

Edit #2: tptacek's responses on the thread are quite interesting, and I agree. These are your friends:

    void *safe_malloc(size_t size) { void *ptr = malloc(size); if(ptr == NULL) abort(); return ptr; }
    #define safe_free(ptr) do { if((ptr) != NULL) { free(ptr); (ptr) = NULL; } } while(0)


Argh, this is so often overlooked. NULL POINTER DEREFERENCE IS A POSSIBLE SECURITY HOLE. Sorry about the shouting, I've gone through this so much I feel like my brain is going to explode. Here, read this: http://lwn.net/Articles/360328/

Don't trust NULL pointer dereference. Even in user code -- it puts you in bad habits.


Where is this magic 1.2GB number coming from?


> Really? How would dereferencing a NULL pointer cause "some really nasty crashes"?

See "Dowd's Inhuman Flash Exploit":

http://evernote.com/pub/petro/random#b=1988c9d5-c0ab-466e-9c...

Maybe see also the various posts on the clang blog about undefined behavior.

> The best and cleanest solution is to crash.

Then write code to crash, you bozo, don't hope the runtime will do it for you. We're not talking about a fucking missile guidance system here:

    void *xmalloc(size_t s) {
            void *rv = malloc(s);
            if (!rv) abort();
            return rv;
    }
xrealloc() is only slightly longer.

Writing and using these functions is not "error prone" or "hard to maintain", and does not "destroy readability". It adds a single letter to each of these function calls and avoids writing any more error-handling code.

That's a completely reasonable solution, but it's not the one you took. It's not the path of least resistance, but when you're programming in C, writing robust code is very rarely the path of least resistance.

With regard to your nonsense about "will only ever happen on 32-bit OSes", I run programs under VM size ulimits every day. This prevents them from forcing the system into swap and becoming unresponsive, by causing their allocations to fail. Some of them (e.g. less) usually handle running out of memory quite well. I happen to be using 32-bit OSes, but that's irrelevant; I would do the same thing on 64-bit OSes.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: