Re: [PATCH v2] diff --no-index: unleak paths[] elements

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Wed, Sep 07 2022, Johannes Schindelin wrote:

> Hi Ævar,
>
> On Mon, 5 Sep 2022, Ævar Arnfjörð Bjarmason wrote:
>
>> On Mon, Sep 05 2022, Johannes Schindelin wrote:
>>
>> [...]
>> > +	string_list_clear(&to_release, 1);
>> [...]
>>
>>  * The free_util=1 in your code isn't needed/is a bug, we make no use of
>>    "util" here, so it should be free_util=0
>
> Calling it a bug is a bit strong[...]

Yes, FWIW I meant that in the sense of "the author probably didn't mean
this" or "it was copy/pasted", but it has no effect currently, as we'll
always have NULL "util" members.

> , and misses the reason why I did it: future-proofing.

I didn't think it was intentional, but obviously you know better about
the intent.

"Future proofing" seems like a bad reason for that API use however. If
you look at the various "util" users some of them want to free() it, and
some don't. If you forget to free() the worst you'll have is a memory
leak, but if you have some boilerplate free() there you'll probably get
a segfault.

Without knowing what the future code looks like we don't know whether it
would make any sense to free() that util use.

> In any case, René took up Junio's ask and provided a new iteration that
> side-steps this concern altogether, therefore the point is now moot.

Indeed, that patch LGTM.