Re: [PATCH 07/49] builtin/repack.c: avoid "the_hash_algo" in `write_oid()`

Taylor Blau
2025-10-07T20:17:22Z

On Tue, Sep 30, 2025 at 01:21:10AM +0200, Patrick Steinhardt wrote:
> On Sun, Sep 28, 2025 at 06:07:37PM -0400, Taylor Blau wrote:
> > diff --git a/builtin/repack.c b/builtin/repack.c
> > index 094f5a0cc2..7d62959dc2 100644
> > --- a/builtin/repack.c
> > +++ b/builtin/repack.c
> > @@ -413,6 +419,7 @@ static void repack_promisor_objects(struct repository *repo,
> > const struct pack_objects_args *args,
> > struct string_list *names)
> > {
> > + struct write_oid_context ctx;
> > struct child_process cmd = CHILD_PROCESS_INIT;
> > FILE *out;
> > struct strbuf line = STRBUF_INIT;
> > @@ -427,7 +434,9 @@ static void repack_promisor_objects(struct repository *repo,
> > * {type -> existing pack order} ordering when computing deltas instead
> > * of a {type -> size} ordering, which may produce better deltas.
> > */
> > - for_each_packed_object(repo, write_oid, &cmd,
> > + ctx.cmd = &cmd;
> > + ctx.algop = repo->hash_algo;
> > + for_each_packed_object(repo, write_oid, &ctx,
> > FOR_EACH_OBJECT_PROMISOR_ONLY);
>
> As this one is essentially new code, and we have all of the info
> available right at the start of the problem. Do we maybe want to use
> designated initializers?

I considered it, but ultimately decided against it here because I find
it clearer to initialize "ctx" right before it is used to avoid
ambiguity.

Alternatively, you could accomplish this with a compound literal, but
that's (a) still a test balloon and (b) more verbose than necessary.

Thanks,
Taylor