Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM

Jeff Garzik <[email protected]> Thu, 23 Sep 2010 14:57:55 -0400
Newsgroups org.kernel.vger.hail-devel
Message-ID <[email protected]>
On 09/23/2010 04:43 AM, Jim Meyering wrote:
>
> Looking at tabled's code, I see a few places where unchecked strdup
> can lead to NULL deref, whereas the majority are checked carefully.
> Patches for the first two I found are below -- haven't completed the job.
>
> In most cases, I see that care is taken to detect failure and propagate
> that to higher levels.  In others, due to the use of glib functions,
> OOM leads to immediate (and possibly-unclean?) exit, because glib simply
> calls exit on OOM.  Or perhaps tabled tells glib not to handle OOM that
> way -- assuming that's possible.
>
> This is server-style (and some of it library-grade) code, so I'm surprised
> to see it relying on glib, which due to its handling of OOM errors
> is often deemed unsuitable for applications that need to die
> gracefully.

It's a holdover from kernel coding that I (and Pete?) obsessively check 
return values, even from memory allocation functions.  Occasionally I 
wonder if I'll ever receive an email, from an odd duck somewhere, 
thanking me for writing a server that works even VM overcommit support 
disabled.

On GLib:  it was just so darned convenient, and portable too.  It gave 
quick access to non-Linux OS's, and a bunch of convenience functions.

GLib's OOM death behavior itself is configurable via g_log*fatal*, but 
you're absolutely right that the code itself makes a standard assumption 
that memory allocation functions always succeed.

I wouldn't complain if the GLib dependency went away, but that's quite a 
project for someone...

(now, on to sending a separate email regarding the specific changes 
you've submitted here...)

	Jeff