"William Baker via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > diff --git a/midx.c b/midx.c > index b2673f52e8..54e4e93b2b 100644 > --- a/midx.c > +++ b/midx.c > @@ -449,6 +449,8 @@ struct pack_list { > uint32_t nr; > uint32_t alloc; > struct multi_pack_index *m; > + struct progress *progress; > + uint32_t pack_paths_checked; > }; What is the reason behind u32 here? Do we want to be consistently limited to 4G on all platforms? I have a feeling that we do not care too deeply all that much for the purpose of progress output, in which case, just an ordinary "unsigned" may be much less misleading. FWIW, the function that uses this field is display_progress(), which takes uint64_t there. > @@ -457,6 +459,7 @@ static void add_pack_to_midx(const char *full_path, size_t full_path_len, > struct pack_list *packs = (struct pack_list *)data; > > if (ends_with(file_name, ".idx")) { > + display_progress(packs->progress, ++packs->pack_paths_checked); OK, "git grep display_progress" tells me that we are expected to pass the value of the counter that is already incremented, so this is also in line with that practice. > + if (flags & MIDX_PROGRESS) > + packs.progress = start_progress(_("Adding packfiles to multi-pack-index"), 0); > + else > + packs.progress = NULL; > > for_each_file_in_pack_dir(object_dir, add_pack_to_midx, &packs); > + stop_progress(&packs.progress); OK. > + if (flags & MIDX_PROGRESS) > + progress = start_progress(_("Writing chunks to multi-pack-index"), > + num_chunks); > ... > + > + display_progress(progress, i + 1); > } > + stop_progress(&progress); > OK.