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

Re: [PATCH v2 4/6] midx-write: use uint32_t for preferred_pack_idx

From
Patrick Steinhardt <ps@pks.im>
Date
Sep 3, 2025, 10:15 UTC
Message-ID
<aLgVOa96grWC3G0Q@pks.im>
In-Reply-To
<2290e27ded03866cb816be5fb6e08b83a75a6f07.1756589007.git.gitgitgadget@gmail.com>
On Sat, Aug 30, 2025 at 09:23:25PM +0000, Derrick Stolee via GitGitGadget wrote:
Show 15 quoted lines
> From: Derrick Stolee <stolee@gmail.com>
> 
> midx-write.c has the DISABLE_SIGN_COMPARE_WARNINGS macro defined for a
> few reasons, but the biggest one is the use of a signed
> preferred_pack_idx member inside the write_midx_context struct. The code
> currently uses -1 to indicate an unset preferred pack but pack int ids
> are normally handled as uint32_t. There are also a few loops that search
> for the preferred pack by name and those iterators will need updates to
> uint32_t in the next change.
> 
> For now, replace the use of -1 with a 'NO_PREFERRED_PACK' macro and an
> equality check. The macro stores the max value of a uint32_t, so we
> cannot store a preferred pack that appears last in a list of 2^32 total
> packs, but that's expected to be unreasonable already. This improves the
> range from 2^31 already.

Tiny nit: the last sentence reads a bit funny. Maybe something like this?

    Furthermore, with this change we end up extending the range from
    2^31 possible packs to 2^32-1.
Show 23 quoted lines
> There are some careful things to worry about with initializing the
> preferred pack in the struct and using that value when searching for a
> preferred pack that was already incorrect but accidentally working when
> the index was initialized to zero.
> 
> Signed-off-by: Derrick Stolee <stolee@gmail.com>
> ---
>  midx-write.c | 26 +++++++++++++++-----------
>  1 file changed, 15 insertions(+), 11 deletions(-)
> 
> diff --git a/midx-write.c b/midx-write.c
> index cb0211289d..1822268ce2 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -274,7 +275,7 @@ static void midx_fanout_add_midx_fanout(struct midx_fanout *fanout,
>  	end = m->num_objects_in_base + ntohl(m->chunk_oid_fanout[cur_fanout]);
>  
>  	for (cur_object = start; cur_object < end; cur_object++) {
> -		if ((preferred_pack > -1) &&
> +		if ((preferred_pack != NO_PREFERRED_PACK) &&
>  		    (preferred_pack == nth_midxed_pack_int_id(m, cur_object))) {
>  			/*
>  			 * Objects from preferred packs are added

Neither of these braces around comparisons are really needed, but feel free to ignore as you simply piggy-back on existing style.

Show 11 quoted lines
> @@ -1040,7 +1042,9 @@ static int write_midx_internal(struct repository *r, const char *object_dir,
>  	struct hashfile *f = NULL;
>  	struct lock_file lk;
>  	struct tempfile *incr;
> -	struct write_midx_context ctx = { 0 };
> +	struct write_midx_context ctx = {
> +		.preferred_pack_idx = NO_PREFERRED_PACK,
> +	 };
>  	int bitmapped_packs_concat_len = 0;
>  	int pack_name_concat_len = 0;
>  	int dropped_packs = 0;

Why is this change needed? We didn't previously initialize `.preferred_pack_idx = -1` either.

Patrick
Previous: Derrick Stolee via GitGitGadgetNext: Derrick Stolee
Message 30 of 47 in “midx-write: fix segfault and do several cleanups”
  1. 0/5 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Aug 28, 2025
  2. 1/5 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Aug 28, 2025
  3. Junio C HamanoAug 28, 2025
  4. Taylor BlauAug 29, 2025
  5. Derrick StoleeAug 30, 2025
  6. 2/5 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Aug 28, 2025
  7. Junio C HamanoAug 28, 2025
  8. Taylor BlauAug 29, 2025
  9. 3/5 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Aug 28, 2025
  10. Junio C HamanoAug 28, 2025
  11. Taylor BlauAug 29, 2025
  12. Derrick StoleeAug 30, 2025
  13. 4/5 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Aug 28, 2025
  14. Junio C HamanoAug 28, 2025
  15. Taylor BlauAug 29, 2025
  16. 5/5 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Aug 28, 2025
  17. Junio C HamanoAug 28, 2025
  18. Taylor BlauAug 29, 2025
  19. Taylor BlauAug 29, 2025
  20. 0/6 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Aug 30, 2025
  21. 1/6 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Aug 30, 2025
  22. Patrick SteinhardtSep 3, 2025
  23. Derrick StoleeSep 5, 2025
  24. 2/6 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Aug 30, 2025
  25. Patrick SteinhardtSep 3, 2025
  26. Derrick StoleeSep 5, 2025
  27. 3/6 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Aug 30, 2025
  28. Patrick SteinhardtSep 3, 2025
  29. 4/6 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Aug 30, 2025
  30. Patrick SteinhardtSep 3, 2025
  31. Derrick StoleeSep 5, 2025
  32. 5/6 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Aug 30, 2025
  33. Patrick SteinhardtSep 3, 2025
  34. 6/6 midx-write: simplify error casesDerrick Stolee via GitGitGadget, Aug 30, 2025
  35. Patrick SteinhardtSep 3, 2025
  36. Junio C HamanoSep 3, 2025
  37. 0/6 midx-write: fix segfault and do several cleanupsDerrick Stolee via GitGitGadget, Sep 5, 2025
  38. 1/6 midx-write: only load initialized packsDerrick Stolee via GitGitGadget, Sep 5, 2025
  39. 2/6 midx-write: put failing response value backDerrick Stolee via GitGitGadget, Sep 5, 2025
  40. 3/6 midx-write: use cleanup when incremental midx failsDerrick Stolee via GitGitGadget, Sep 5, 2025
  41. 4/6 midx-write: use uint32_t for preferred_pack_idxDerrick Stolee via GitGitGadget, Sep 5, 2025
  42. 5/6 midx-write: reenable signed comparison errorsDerrick Stolee via GitGitGadget, Sep 5, 2025
  43. 6/6 midx-write: simplify error casesDerrick Stolee via GitGitGadget, Sep 5, 2025
  44. Junio C HamanoSep 5, 2025
  45. Derrick StoleeSep 5, 2025
  46. Taylor BlauSep 11, 2025
  47. Junio C HamanoSep 11, 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.