git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 3/4] midx: avoid negative array index

From
Taylor Blau <me@ttaylorr.com>
Date
May 20, 2025, 17:58 UTC
Message-ID
<aCzCsv61ET4u7JOK@nand.local>
In-Reply-To
<688b0273604179b5bebe3748445158e09a7bf1a0.1747753388.git.phillip.wood@dunelm.org.uk>
On Tue, May 20, 2025 at 04:04:26PM +0100, Phillip Wood wrote:
Show 39 quoted lines
> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>
> nth_midxed_pack_int_id() returns the index of the pack file in the multi
> pack index's list of packfiles that the specified object. The index is
> returned as a uint32_t. Storing this in an int will make the index
> negative if the most significant bit is set. Fix this by using uint32_t
> as the rest of the code does. This is unlikely to be a practical problem
> as it requires the multipack index to reference 2^31 packfiles.
>
> Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
> ---
>  midx-write.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/midx-write.c b/midx-write.c
> index 2ee381e8fcd..38a458d7322 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -1566,7 +1566,7 @@ int expire_midx_packs(struct repository *r, const char *object_dir, unsigned fla
>  					  _("Counting referenced objects"),
>  					  m->num_objects);
>  	for (i = 0; i < m->num_objects; i++) {
> -		int pack_int_id = nth_midxed_pack_int_id(m, i);
> +		uint32_t pack_int_id = nth_midxed_pack_int_id(m, i);
>  		count[pack_int_id]++;
>  		display_progress(progress, i + 1);
>  	}
> @@ -1697,7 +1697,7 @@ static void fill_included_packs_batch(struct repository *r,
>
>  	total_size = 0;
>  	for (i = 0; total_size < batch_size && i < m->num_packs; i++) {
> -		int pack_int_id = pack_info[i].pack_int_id;
> +		uint32_t pack_int_id = pack_info[i].pack_int_id;
>  		struct packed_git *p = m->packs[pack_int_id];
>  		uint64_t expected_size;
>
> --
> 2.49.0.897.gfad3eb7d210
>

Thanks for catching these. I was going to comment on it as something I noticed while reading the first patch, but I'm glad that you addressed it here.

It looks like these two declarations date back to:
 - 19575c7c8e (multi-pack-index: implement 'expire' subcommand, 2019-06-10)
 - ce1e4a105b (midx: implement midx_repack(), 2019-06-10)

(both of which were merged via 4308d81d45 (Merge branch 'ds/midx-expire-repack', 2019-07-19)). But I think these have always had the wrong type, since the pack_int_id field comes from commit fe1ed56f5e (midx: sort and deduplicate objects from packfiles, 2018-07-12), where it was first introduced as a uint32_t.

I actually think these all should size_t's since they're indexing into an array, but that can be dealt with outside of this series, since this is an obvious improvement.

Thanks, Taylor

Previous: Phillip WoodNext: Phillip Wood
Message 13 of 25 in “midx repack: fix overflow on 32 bit systems”
  1. 0/4 midx repack: fix overflow on 32 bit systemsPhillip Wood, May 20, 2025
  2. 1/4 midx repack: avoid integer overflow on 32 bit systemsPhillip Wood, May 20, 2025
  3. Taylor BlauMay 20, 2025
  4. Phillip WoodMay 21, 2025
  5. Taylor BlauMay 23, 2025
  6. D. Ben KnobleMay 21, 2025
  7. Junio C HamanoMay 21, 2025
  8. Phillip WoodMay 21, 2025
  9. 2/4 midx repack: avoid potential integer overflow on 64 bit systemsPhillip Wood, May 20, 2025
  10. Taylor BlauMay 20, 2025
  11. Phillip WoodMay 21, 2025
  12. 3/4 midx: avoid negative array indexPhillip Wood, May 20, 2025
  13. Taylor BlauMay 20, 2025
  14. 4/4 midx docs: clarify tie breakingPhillip Wood, May 20, 2025
  15. Taylor BlauMay 20, 2025
  16. Phillip WoodMay 21, 2025
  17. D. Ben KnobleMay 21, 2025
  18. 0/4 midx repack: fix overflow on 32 bit systemsPhillip Wood, May 22, 2025
  19. 2/4 midx repack: avoid potential integer overflow on 64 bit systemsPhillip Wood, May 22, 2025
  20. 1/4 midx repack: avoid integer overflow on 32 bit systemsPhillip Wood, May 22, 2025
  21. 3/4 midx: avoid negative array indexPhillip Wood, May 22, 2025
  22. 4/4 midx docs: clarify tie breakingPhillip Wood, May 22, 2025
  23. Taylor BlauMay 23, 2025
  24. Phillip WoodMay 27, 2025
  25. Junio C HamanoMay 27, 2025

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.