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

From: Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com>
Date: Apr 6, 2026, 18:40 UTC
Message-ID: <adP-MYYSmElK9wL3@lorenzo-VM>
In-Reply-To: <f2540dc1-fe76-48ec-91a7-82e32ced75fc@malon.dev>

On Tue, Apr 07, 2026 at 01:22:16AM +0800, Tian Yuchen wrote:

Show 18 quoted lines

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

Show 7 quoted lines

> > +
> > +			/* If <time> doesn't exist, retrieve it and add it to line */
> > +			if (!parts[2]) {
> > +				struct tm tm;
> > +				localtime_r(&source_stat.st_mtim.tv_sec, &tm),
> 
> Typo.

Ack.

Show 17 quoted lines

> 
> > +				strbuf_addch(&line, ' ');
> > +				strbuf_addftime(&line, "%Y/%m/%d-%H:%M:%S", &tm, 0, 0);
> > +			}
> > +
> > +			/*
> > +			 * Add the finalized line to dest_to_write and dest_content if it
> > +			 * wasn't already present inside dest_content
> > +			 */
> > +			if (strset_add(&dest_content, line.buf)) {
> > +				strbuf_addbuf(&dest_to_write, &line);
> > +				strbuf_addch(&dest_to_write, '\n');
> > +			}
> 
> It looks good elsewhere, at least in this patch 2/5. (ゝ∀･)
> 
> Regards, Yuchen

Thank you Yuchen!

Lorenzo