Re: [PATCH 6/6] reflog-walk: stop using fake parents

From: Jeff King <peff@peff.net>
Date: Jul 7, 2017, 03:15 UTC
Message-ID: <20170707031508.kujsp6lsbgxdygvq@sigill.intra.peff.net>
In-Reply-To: <20170707030224.4iubuzsz2qwdhgiw@sigill.intra.peff.net>

On Thu, Jul 06, 2017 at 11:02:24PM -0400, Jeff King wrote:

Show 31 quoted lines

> On Fri, Jul 07, 2017 at 12:32:39AM +0000, Eric Wong wrote:
> 
> > I'm not sure why, but this is causing t1414.8 failures on 32-bit
> > x86 with the latest pu with Debian jessie (oldstable).
> > 
> > Reverting this (beafb2c62947a6d4a97b9c3baf99fe62ec8e830f) in pu
> > seems to fix the test for me.
> > 
> > +Cc: Ramsay since he also had a 32-bit environment.
> 
> Thanks, I was able to reproduce with CFLAGS=-m32.
> 
> > --- expect	2017-07-07 00:30:57.796325232 +0000
> > +++ actual	2017-07-07 00:30:57.796325232 +0000
> > @@ -3,6 +3,8 @@
> >  HEAD@{2} checkout: moving from master to side
> >  HEAD@{3} commit: two
> >  HEAD@{4} commit (initial): one
> > -side@{0} commit (merge): Merge branch 'master' into side
> > -side@{1} commit: three
> > -side@{2} branch: Created from HEAD^
> > +HEAD@{0} commit (merge): Merge branch 'master' into side
> > +HEAD@{1} commit: three
> > +HEAD@{2} checkout: moving from master to side
> > +HEAD@{3} commit: two
> > +HEAD@{4} commit (initial): one
> 
> That's quite an unexpected error (to show the HEAD reflog twice). Given
> that it triggers with 32-bit builds, it's like there's some funny memory
> error. And indeed, running it under valgrind shows a problem in
> add_reflog_for_walk. I'll try to dig into it. Thanks for reporting.

Ah, it's a bug in the existing code. It inserts an entry into a string list and then frees the memory, but the string list wasn't asked to make a copy of the string. You can see the bug by running t1414 with --valgrind even before any of the code changes (though note that you'll have to look at the "-v" output, since it's already marked as expect-failure).

This fixes it:

diff --git a/reflog-walk.c b/reflog-walk.c
index b7e489ad32..c1f61c524a 100644
--- a/reflog-walk.c
+++ b/reflog-walk.c
@@ -136,6 +136,7 @@ struct reflog_walk_info {
 void init_reflog_walk(struct reflog_walk_info **info)
 {
 	*info = xcalloc(1, sizeof(struct reflog_walk_info));
+	(*info)->complete_reflogs.strdup_strings = 1;
 }
 
 int add_reflog_for_walk(struct reflog_walk_info *info,

I'll add that to the 'maint' portion of the series.

-Peff