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

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 1, 2022, 19:32 UTC
Message-ID: <xmqq35iwwqe9.fsf@gitster.g>
In-Reply-To: <220401.868rsoogxf.gmgdl@evledraar.gmail.com>

Ævar Arnfjörð Bjarmason <avarab@gmail.com> 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.

Show 6 quoted lines

> 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.