Re: [PATCH 2/3] setup: have the_repository use the_index

[Junio C Hamano <gitster@pobox.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Jonathan Nieder <jrnieder@gmail.com> 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.