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
next prev parent 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