Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX

From: Jeff King <peff@peff.net>
Date: Sep 6, 2017, 13:35 UTC
Message-ID: <20170906133519.amlc5yunx4yfl5u3@sigill.intra.peff.net>
In-Reply-To: <6a4cbbee-ffc6-739b-d649-079ba01439ca@grubix.eu>

On Wed, Sep 06, 2017 at 01:59:31PM +0200, Michael J Gruber wrote:

Show 19 quoted lines

> BTW, there's more fallout from those name-rev changes: In connection
> with that other thread about surprising describe results for emacs.git I
> noticed that I can easily get "git name-rev --stdin" to segfault there.
> As easy as
> 
> echo bc5d96a0b2a1dccf7eeeec459e40d21b54c977f4 | git name-rev --stdin
> 
> for example.
> 
> That's unfortunate for the use-case of name-rev to amend git log output.
> 
> The reason seems to be that with "--stdin" or "--all", "name-rev" walks
> and names all commits before beginning to use that those names for even
> a single commit as above.
> 
> That segfault bisects to the logic changing commit in
> jc/name-rev-lw-tag, but I think the changed logic simply leads to more
> xmallocs() the segfault sooner now. Or something that I dind't spot even
> after a few hours.

The segfault seems to be due to running out of stack space. The problem is that name_rev() is recursive over the history graph. That topic added a parameter to the function, which increased the memory used for each level of the recursion. But the fundamental problem has always been there. The right solution is to switch to iteration (with our own stack structure if necessary).

We had similar problems with the recursive --contains traversal in tag, and ended up with cbc60b6720 (git tag --contains: avoid stack overflow, 2014-04-24).

-Peff