Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
- Date: Wed, 6 Sep 2017 09:35:19 -0400
- From: Jeff King <peff@xxxxxxxx>
- Subject: Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
On Wed, Sep 06, 2017 at 01:59:31PM +0200, Michael J Gruber wrote:
> 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,