Subject: Re: [GSoC PATCH v3 2/5] pack-write: add helper to fill promisor file after repack

From: Junio C Hamano <[email-masked]>
Date: Mon, 06 Apr 2026 14:17:02 -0700

Lorenzo Pegorari <[email-masked]> writes:

> On Tue, Apr 07, 2026 at 01:22:16AM +0800, Tian Yuchen wrote:
>> Hi,
>> 
>> On 4/6/26 08:24, LorenzoPegorari wrote:
>> 
>> > +		while (strbuf_getline(&line, source) != EOF) {
>> > +			struct strbuf **parts;
>> > +			struct object_id oid;
>> > +
>> > +			/* Split line into <oid>, <ref> and <time> (if <time> exists) */
>> > +			parts = strbuf_split_max(&line, ' ', 3);
>> > +
>> > +			/* Ignore the lines where <oid> doesn't appear in the dest_pack */
>> > +			strbuf_rtrim(parts[0]);
>> > +			get_oid_hex_algop(parts[0]->buf, &oid, repo->hash_algo);
>> > +			if (!find_pack_entry_one(&oid, dest_pack))
>> > +				continue;
>> 
>> Memory leak here;
>
> Yep, `strbuf_list_free(parts)` is missing here. Ack.

Also strbuf_split*() is a bad API.  Unless you need all the parts[]
strbuf instances all editable at the same time, an array of strbuf
is a data structure that is way overkill.  Splitting into string-list
may make it more palatable, I think.

We even went through a series of patches (and follow-up effort by
other contributors) [*] to rewrite callers that unnecessarily call
strbuf_split*().

[References]
  https://lore.kernel.org/git/[email-masked]/
  https://lore.kernel.org/git/[email-masked]/