Re: [PATCH] use xstrdup, not strdup in ll-merge.c

From: Shawn O. Pearce <spearce@spearce.org>
Date: Jun 15, 2009, 14:26 UTC
Message-ID: <20090615142648.GV16497@spearce.org>
In-Reply-To: <81b0412b0906150546p34ff53ceo81ea59785cf9d18a@mail.gmail.com>

Alex Riesen <raa.lkml@gmail.com> wrote:

Show 6 quoted lines

> 2009/6/15 Jim Meyering <jim@meyering.net>:
> > Alex Riesen wrote:
> >> 2009/6/15 Jim Meyering <jim@meyering.net>:
> >>>
> >>> Exactly. ??This is why I think it's not worthwhile to invest in
> >>> a more precise diagnostic, here.

Sorry Alex, but I have to agree with Jim here. Use xstrdup() and its friends and move on with life.

Show 5 quoted lines

> As much as I'd like to know as much as possible about why something
> failed, I can't make a failure handling automatically simple (given the tools).
> I do suggest using goto to handle this particular OOM (below), but it looks
> almost too ugly. Maybe I am just paranoid, and am overdoing this particular
> case.

Yes, this is horrible. Its a huge amount of code for checking a strdup failure that is unlikely to occur out of a configuration file.

Really, how big is that config file? It can't be so large that a strdup() is likely to run out of memory pulling a value from it.

And even if it does run out of memory, unless we can reclaim space
by shedding pack windows, we're doomed, we can't continue executing.
The official error is "out of memory", most likely due to too low
of a rlimit on our heap, and the only resolution is to boost the
rlimit higher, not something we can do in our process if we're
already hitting the hard limit.

> The "goto oom" patch:

NAK.  We don't do this anywhere else in git.  Please don't start now.

-- 
Shawn.