All of lore.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.