Subject: Re: [PATCH v4 04/27] format-patch: don't leak "extra_headers" or "ref_message_ids"

From: Junio C Hamano <[email-masked]>
Date: Fri, 01 Apr 2022 12:32:46 -0700

Ævar Arnfjörð Bjarmason <[email-masked]> writes:

> The below patch fails tests, it's just a quick (and obviously flawed)
> search/replacement that I hacked up to get the removal of these from
> revision.h to compile, which shows that it's just something between
> log.c and log-tree.c, pretty much.

Given that the way most end-users see log-tree.c in action is via
log.c (is there a useful entry point into log-tree that bypasses
log.c that implements end-user actions?), I would actually think it
is that extra_headers is quite an integral part of rev_info
structure, which is how the revision traversal API passes
information relevant to the current traversal around various layors
of the implementation.  So, I am not sure what the above experiment
shows us.

> Anyway, while I'm 100% in agreement with you that this *should* be fixed
> I'd really like to do the bare minimum to address leaks in this initial
> iteration.
>
> I.e. you're right that this relatively fragile, and I'm also concern
> about catching memory errors.

As long as we know there are yet more work to do (are we leaving
NEEDSWORK comments as we go, by the way?), I think that's fine.  We
cannot go all the way in one go for a large code base like revisions
API, and we can leave a small corner "not quite optimial", and we can
even leave it in "sometimes still leaks" status, as long as we never
make anything to free too much by mistake.