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
