git/list[1] front-page[2] threads[3] people[4] search[5] about
wed 2026-10-07 18:05 UTC

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.

Previous: Junio C HamanoNext: Junio C Hamano
Message 31 of 79 in “preserve promisor files content after repack”
  1. 0/3 preserve promisor files content after repackLorenzoPegorari, Mar 21, 2026
  2. 1/3 pack-write: add explanation to promisor file contentLorenzoPegorari, Mar 21, 2026
  3. 2/3 pack-write: add helper to fill promisor file after repackLorenzoPegorari, Mar 21, 2026
  4. 3/3 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Mar 21, 2026
  5. Eric SunshineMar 22, 2026
  6. Lorenzo PegorariMar 22, 2026
  7. 0/4 preserve promisor files content after repackLorenzoPegorari, Mar 22, 2026
  8. 1/4 pack-write: add explanation to promisor file contentLorenzoPegorari, Mar 22, 2026
  9. 2/4 pack-write: add helper to fill promisor file after repackLorenzoPegorari, Mar 22, 2026
  10. 3/4 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Mar 22, 2026
  11. 4/4 t7700: test for promisor file content after repackLorenzoPegorari, Mar 22, 2026
  12. Eric SunshineMar 23, 2026
  13. Junio C HamanoMar 23, 2026
  14. Junio C HamanoMar 23, 2026
  15. Junio C HamanoMar 23, 2026
  16. Lorenzo PegorariMar 25, 2026
  17. Lorenzo PegorariMar 26, 2026
  18. Lorenzo PegorariMar 26, 2026
  19. Lorenzo PegorariMar 26, 2026
  20. 0/5 preserve promisor files content after repackLorenzoPegorari, Apr 6, 2026
  21. 1/5 pack-write: add explanation to promisor file contentLorenzoPegorari, Apr 6, 2026
  22. 2/5 pack-write: add helper to fill promisor file after repackLorenzoPegorari, Apr 6, 2026
  23. 3/5 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Apr 6, 2026
  24. 4/5 t7700: test for promisor file content after repackLorenzoPegorari, Apr 6, 2026
  25. 5/5 t7703: test for promisor file content after geometric repackLorenzoPegorari, Apr 6, 2026
  26. Tian YuchenApr 6, 2026
  27. Lorenzo PegorariApr 6, 2026
  28. Junio C HamanoApr 6, 2026
  29. Junio C HamanoApr 6, 2026
  30. Junio C HamanoApr 6, 2026
  31. Junio C HamanoApr 7, 2026
  32. Junio C HamanoApr 7, 2026
  33. Lorenzo PegorariApr 7, 2026
  34. Lorenzo PegorariApr 7, 2026
  35. Junio C HamanoApr 7, 2026
  36. Lorenzo PegorariApr 7, 2026
  37. Lorenzo PegorariApr 7, 2026
  38. Lorenzo PegorariApr 7, 2026
  39. Lorenzo PegorariApr 8, 2026
  40. 0/5 preserve promisor files content after repackLorenzoPegorari, Apr 10, 2026
  41. 1/5 pack-write: add explanation to promisor file contentLorenzoPegorari, Apr 10, 2026
  42. 2/5 pack-write: add helper to fill promisor file after repackLorenzoPegorari, Apr 10, 2026
  43. 3/5 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Apr 10, 2026
  44. 4/5 t7700: test for promisor file content after repackLorenzoPegorari, Apr 10, 2026
  45. 5/5 t7703: test for promisor file content after geometric repackLorenzoPegorari, Apr 10, 2026
  46. Junio C HamanoApr 10, 2026
  47. Junio C HamanoApr 10, 2026
  48. Lorenzo PegorariApr 10, 2026
  49. Lorenzo PegorariApr 10, 2026
  50. CodingGuidelines: st_mtimespec vs st_mtim vs st_mtimeJunio C Hamano, Apr 10, 2026
  51. 0/6 preserve promisor files content after repackLorenzoPegorari, Apr 10, 2026
  52. 1/6 pack-write: add explanation to promisor file contentLorenzoPegorari, Apr 10, 2026
  53. 2/6 repack-promisor add helper to fill promisor file after repackLorenzoPegorari, Apr 10, 2026
  54. 3/6 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Apr 10, 2026
  55. 4/6 t7700: test for promisor file content after repackLorenzoPegorari, Apr 10, 2026
  56. 5/6 t7703: test for promisor file content after geometric repackLorenzoPegorari, Apr 10, 2026
  57. 6/6 repack-promisor: add missing headersLorenzoPegorari, Apr 10, 2026
  58. Junio C HamanoApr 10, 2026
  59. Junio C HamanoApr 10, 2026
  60. Lorenzo PegorariApr 11, 2026
  61. Junio C HamanoApr 11, 2026
  62. Lorenzo PegorariApr 11, 2026
  63. Tian YuchenApr 11, 2026
  64. Tian YuchenApr 11, 2026
  65. Junio C HamanoApr 12, 2026
  66. Elijah NewrenApr 16, 2026
  67. Lorenzo PegorariApr 17, 2026
  68. Lorenzo PegorariApr 17, 2026
  69. Lorenzo PegorariApr 17, 2026
  70. Junio C HamanoApr 17, 2026
  71. 0/6 preserve promisor files content after repackLorenzoPegorari, Apr 18, 2026
  72. 1/6 pack-write: add explanation to promisor file contentLorenzoPegorari, Apr 18, 2026
  73. 2/6 repack-promisor add helper to fill promisor file after repackLorenzoPegorari, Apr 18, 2026
  74. 3/6 repack-promisor: preserve content of promisor files after repackLorenzoPegorari, Apr 18, 2026
  75. 4/6 t7700: test for promisor file content after repackLorenzoPegorari, Apr 18, 2026
  76. 5/6 t7703: test for promisor file content after geometric repackLorenzoPegorari, Apr 18, 2026
  77. 6/6 repack-promisor: add missing headersLorenzoPegorari, Apr 18, 2026
  78. Junio C HamanoMay 12, 2026
  79. Lorenzo PegorariMay 19, 2026

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.