From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jeff Garzik Subject: Re: [PATCH tabled 1/2] server/config.c: don't dereference NULL on OOM Date: Thu, 23 Sep 2010 14:57:55 -0400 Message-ID: <4C9BA333.4090404@garzik.org> References: <87zkv8yhnu.fsf@meyering.net> Mime-Version: 1.0 Content-Transfer-Encoding: 7bit Return-path: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:sender:message-id:date:from :user-agent:mime-version:to:cc:subject:references:in-reply-to :content-type:content-transfer-encoding; bh=9GvMKFSKWZi52hkBSdmFj/eYKItvC5fu9r3hEhUMEks=; b=xtrqewHZayxdsK9R2azksKyCagESxG0X5rtTkT3F43eX+gFy72GX6elF/7oKzA6T2+ vvTRUtPBiEJBjBIZnPoIODqoAxOHLPk5cIWqAQc7Nfn0jAxvOYxR2h3v1vPdSiE3d/0W AYa/I+jrDOPXDgFJoM7bZi9Repi1XZewxBYrI= In-Reply-To: <87zkv8yhnu.fsf@meyering.net> Sender: hail-devel-owner@vger.kernel.org List-ID: Content-Type: text/plain; charset="us-ascii"; format="flowed" To: Jim Meyering Cc: Project Hail 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