public inbox for hail-devel@vger.kernel.org
 help / color / mirror / Atom feed
From: Jeff Garzik <jeff@garzik.org>
To: Jim Meyering <jim@meyering.net>
Cc: Project Hail <hail-devel@vger.kernel.org>
Subject: Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM
Date: Thu, 23 Sep 2010 14:57:55 -0400	[thread overview]
Message-ID: <4C9BA333.4090404@garzik.org> (raw)
In-Reply-To: <87zkv8yhnu.fsf@meyering.net>

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


  reply	other threads:[~2010-09-23 18:57 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-09-23  8:43 [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM Jim Meyering
2010-09-23 18:57 ` Jeff Garzik [this message]
2010-09-23 19:19 ` Jeff Garzik
2010-09-23 19:35   ` Jeff Garzik
2010-09-24 11:32     ` Jim Meyering
2010-09-24 11:55       ` Jim Meyering
2010-09-24 17:24       ` Jeff Garzik
2010-09-24 17:43         ` Jim Meyering
2010-09-24 17:50           ` Jeff Garzik
2010-09-24 11:02   ` Jim Meyering

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4C9BA333.4090404@garzik.org \
    --to=jeff@garzik.org \
    --cc=hail-devel@vger.kernel.org \
    --cc=jim@meyering.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox