On 1/10/2023 1:17 AM, Ævar Arnfjörð Bjarmason wrote:
> When the "repo" member was added to "the_index" in [1] the
> repo_read_index() was made to populate it, but the unpopulated
> "the_index" variable didn't get the same treatment.
>
> Let's do that in initialize_the_repository() when we set it up, and
> likewise for all of the current callers initialized an empty "struct
> index_state".
> + struct index_state result = { .repo = state->repo };
> + struct index_state wtindex = { .repo = the_repository };
> + o.result.repo = r;
> + struct index_state istate = { .repo = the_repository };
I think these initialization updates (along with the others I didn't
include) are satisfactory for now. What worries me is that future
consumers that create an index_state will need to remember to manually
initialize like this or risk hitting the BUG() statements in
read-cache.c.
The only alternative I can think about is to create an initialization
method, say "init_index_state(struct index_state *, struct repository *)",
that should be called before doing anything with an index_state. This
includes running a memset() to clear the struct, making these inline
initializers unnecessary.
However, I can't decide if that's actually an improvement. I think
things tip in favor of the init_index_state() method if there ever
becomes another member of struct index_state that _needs_ to be set
before the struct is "valid". I doubt that we would add such a thing
in the near future, so I recommend sticking with this patch as-is.
Thanks,
-Stolee
