"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: fix 'got blame' segfault
To:
Josh Rickmar <joshrickmar@outlook.com>
Cc:
gameoftrees@openbsd.org
Date:
Fri, 18 Dec 2020 13:18:48 +0100

Download raw body.

Thread
On Thu, Dec 17, 2020 at 07:59:16PM -0500, Josh Rickmar wrote:
> On Thu, Dec 17, 2020 at 09:35:46PM +0100, Stefan Sperling wrote:
> > On IRC jrick reported that this command triggers a segfault:
> > 
> >   $ got blame distrib/miniroot/install.sub
> > 
> >   assertion "blame->f2" failed: file "/home/jrick/src/got/tog/../lib/blame.c", line 480, function "close_file2_and_reuse_file1"
> > 
> > In fact, both file pointers in the 'blame' struct are NULL.
> > 
> > The problem is that the commit graph traverses one commit too far.
> > This can be more easily reproduced with 'got log':
> > 
> >  got log -c 05f568 -P distrib/miniroot/install.sub
> > 
> > This lists two commits:
> >   05f568ecc6aadefa1aff9064a29e798874a71409
> >   7c0d87f00e480cdf004324dad6f3e6f4418f8f42 
> > 
> > commit 05f568ecc6aade... added the file distrib/miniroot/install.sub.
> > The above 'got log' command returns not just this commit as expected,
> > it additionally returns commit 7c0d87f00e480cd...
> > 
> > The problem is a bug in the logic of got_object_tree_path_changed(),
> > which causes the commit graph to add 7c0d87f00e480...  to the list of
> > commits which must still be traversed, even though the history of the
> > logged path has already ended.
> > 
> > Note that "distrib/miniroot" exists in 7c0d87f00e480cdf004324... but not
> > in the parent of this commit.
> > 
> > At the point where are traversing "distrib/miniroot" the tree entry te1
> > exists and corresponds to this tree listing the contents of 'miniroot'
> > in 7c0d87f00e480cdf004324...:
> > 
> > $ got cat 180aa33df8d1
> 
> Where does object come from?

It is the tree object for the path 'distrib/miniroot' in commit 7c0d87f:

$ got tree -c 7c0d87f -i distrib | grep miniroot
180aa33df8d134a83f10591911c91dc439db8c76 miniroot/

> Other than the question above (which is just me trying to understand
> how this was debugged), this makes sense to me and the patch looks
> right.  I confirmed that the segfault and the debug assert i added is
> no longer being hit, and got log -P only shows the 05f568ecc6aade
> commit.

Thanks. So with the above question addressed, this is ok by you?