On Thu, May 23, 2024 at 12:38:06PM -0400, Taylor Blau wrote:
> diff --git a/midx-write.c b/midx-write.c
> index 03e95ae821..ad32e8953d 100644
> --- a/midx-write.c
> +++ b/midx-write.c
> @@ -299,21 +299,17 @@ static void midx_fanout_add_pack_fanout(struct midx_fanout *fanout,
> * Copy only the de-duplicated entries (selected by most-recent modified time
> * of a packfile containing the object).
> */
> -static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
> - struct pack_info *info,
> - uint32_t nr_packs,
> - size_t *nr_objects,
> - int preferred_pack)
> +static struct pack_midx_entry *get_sorted_entries(struct write_midx_context *ctx)
> {
> uint32_t cur_fanout, cur_pack, cur_object;
> size_t alloc_objects, total_objects = 0;
> struct midx_fanout fanout = { 0 };
> struct pack_midx_entry *deduplicated_entries = NULL;
> - uint32_t start_pack = m ? m->num_packs : 0;
> + uint32_t start_pack = ctx->m ? ctx->m->num_packs : 0;
>
> - for (cur_pack = start_pack; cur_pack < nr_packs; cur_pack++)
> + for (cur_pack = start_pack; cur_pack < ctx->nr; cur_pack++)
> total_objects = st_add(total_objects,
> - info[cur_pack].p->num_objects);
> + ctx->info[cur_pack].p->num_objects);
>
> /*
> * As we de-duplicate by fanout value, we expect the fanout
> @@ -324,25 +320,25 @@ static struct pack_midx_entry *get_sorted_entries(struct multi_pack_index *m,
>
> ALLOC_ARRAY(fanout.entries, fanout.alloc);
> ALLOC_ARRAY(deduplicated_entries, alloc_objects);
> - *nr_objects = 0;
> + ctx->entries_nr = 0;
Nit: I think it's a bit surprising that a getter function would modify
the passed in structure. It's also a bit puzzling that we assign
`entries_nr` in here, but rely on the caller to set the corresponding
`entries` field. I think we should either have the caller assign both
fields, or we should rename the function and assign both of these fields
in the function.
Patrick
Attachment:
signature.asc
Description: PGP signature
