Junio C Hamano <[email protected]> writes:
> Jonathan Nieder <[email protected]> writes:
>
>>
>> All that said, I don't have a strong opinion on this. Both the 1-word
>> approach (a pointer) and 24-word approach (embedding) are tolerable
>> and there are reasons to prefer each.
>
> I do not care too much about 24-word wastage. If this were not "a
> pointer pretending to be embedded object", the fix in 1/3 wouldn't
> have been necessary. I am worried about this being an invitations
> for such unnecesasry bugs.
Another thing I noticed that you already pointed out was this bit in
your review message:
> I wonder if this can be done sooner. For example, does the following
> work? This way, 'the_repository->index == &the_index' would be an
> invariant that always holds, even in the early setup stage before
> setup_git_directory_gently has run completely.
>
> Thanks,
> Jonathan
>
> diff --git i/repository.c w/repository.c
> index edca907404..bdc1f93282 100644
> --- i/repository.c
> +++ w/repository.c
> @@ -4,7 +4,7 @@
> #include "submodule-config.h"
>
> /* The main repository */
> -static struct repository the_repo;
> +static struct repository the_repo = { .index = &the_index };
> struct repository *the_repository = &the_repo;
>
> static char *git_path_from_env(const char *envvar, const char *git_dir,
With a pointer that can point at a random instance of index_state,
the current "struct repository" allows two or more instances of it
to share the same index_state. I do not think that is a designed
and a desirable "feature" but an invitation for a mistake.
Embedding the real instance in it would solve that, too.
So, after saying "I am not (yet) telling you to fix the design" and
then hearing what a potential advanage could be (and none of that
was a convincing one), I am inclined to say that this eventually
needs to be fixed, preferrably before too much code starts relying
on it and making it more work to fix it later.
