Re: [PATCH v3 3/6] log: fix a memory leak in "git show <revision>..."

[Jeff King <peff@peff.net>]

On Wed, Aug 03, 2022 at 01:27:36PM -0400, Jeff King wrote:

> > diff --git a/builtin/log.c b/builtin/log.c
> > index 88a5e98875a..b4b1d974617 100644
> > --- a/builtin/log.c
> > +++ b/builtin/log.c
> > @@ -743,11 +743,17 @@ int cmd_show(int argc, const char **argv, const char *prefix)
> >  			rev.shown_one = 1;
> >  			break;
> >  		case OBJ_COMMIT:
> > +		{
> > +			struct object_array old;
> > +
> > +			memcpy(&old, &rev.pending, sizeof(old));
> >  			rev.pending.nr = rev.pending.alloc = 0;
> >  			rev.pending.objects = NULL;
> >  			add_object_array(o, name, &rev.pending);
> >  			ret = cmd_log_walk_no_free(&rev);
> > +			memcpy(&rev.pending, &old, sizeof(rev.pending));
> >  			break;
> > +		}
> 
> Here we overwrite the one-item rev.pending without freeing it, but just
> immediately after instead of before. It's a little subtle, but your
> comment in the commit message:
> 
>   [...] and only free the new "rev.pending" in the "OBJ_COMMIT" case arm
>   as prepare_revision_walk() would draw it down.
> 
> covers that. IMHO that could be spelled out a bit more (particularly
> that this only works for OBJ_COMMIT, but that's OK because that's the
> only type we're adding), but I can live with it.

Ah, I see that you did add a comment in the next commit. That's
sufficient, though I really think it makes more sense here, where we're
actually dealing with leaks.

-Peff