Re: [GSoC PATCH v3 2/5] pack-write: add helper to fill promisor file after repack
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Apr 7, 2026, 02:01 UTC
- Message-ID
- <xmqqy0iz7clt.fsf@gitster.g>
- In-Reply-To
- <adP-MYYSmElK9wL3@lorenzo-VM>
Lorenzo Pegorari <lorenzo.pegorari2002@gmail.com> writes:
Show 31 quoted lines
> 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.
>
>> > +
>> > + /* 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.Not just an unintended use of comma operator, this is not portable and breaks OSX build
https://github.com/git/git/actions/runs/24058681172/job/70170218891#step:4:213
>> > + strbuf_addch(&line, ' '); >> > + strbuf_addftime(&line, "%Y/%m/%d-%H:%M:%S", &tm, 0, 0);
I suspect that storing seconds since epoch as a large integer would be simpler and much less error prone than storing localtime in textual form without even recording the timezone.