git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jeff King <peff@peff.net>
To: Michael J Gruber <michael@grubix.eu>
Cc: Junio C Hamano <gitster@pobox.com>,
	git@vger.kernel.org,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>
Subject: Re: [PATCH] name-rev: change ULONG_MAX to TIME_MAX
Date: Wed, 6 Sep 2017 09:35:19 -0400	[thread overview]
Message-ID: <20170906133519.amlc5yunx4yfl5u3@sigill.intra.peff.net> (raw)
In-Reply-To: <6a4cbbee-ffc6-739b-d649-079ba01439ca@grubix.eu>

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,
2014-04-24).

-Peff

  reply	other threads:[~2017-09-06 13:35 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-30  9:46 [PATCH] name-rev: change ULONG_MAX to TIME_MAX Michael J Gruber
2017-08-30 20:23 ` Johannes Schindelin
2017-09-06  3:35 ` Junio C Hamano
2017-09-06 11:59   ` Michael J Gruber
2017-09-06 13:35     ` Jeff King [this message]
2017-09-07 12:17       ` Michael J Gruber
2017-09-07 14:02         ` [PATCH 0/4] Test name-rev with small stack Michael J Gruber
2017-09-07 14:02           ` [PATCH 1/4] t7004: move limited stack prereq to test-lib Michael J Gruber
2017-09-07 14:02           ` [PATCH 2/4] t6120: test name-rev --all and --stdin Michael J Gruber
2017-09-07 14:02           ` [PATCH 3/4] t6120: clean up state after breaking repo Michael J Gruber
2017-09-07 14:02           ` [PATCH 4/4] t6120: test describe and name-rev with deep repos Michael J Gruber
2017-09-07 14:54           ` [PATCH 0/4] Test name-rev with small stack Jeff King
2017-09-08 12:33             ` Michael J Gruber
2017-09-11 18:08               ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170906133519.amlc5yunx4yfl5u3@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=michael@grubix.eu \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).