Hi,
Matheus Tavares Bernardino wrote:
> Sorry for the late reply, last week was quite busy.
No problem. It's an unusual time for everyone.
[...]
> On Wed, Sep 2, 2020 at 5:15 PM Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
>> @@ -1824,27 +1824,33 @@ void read_early_config(config_fn_t cb, void *data)
>> struct config_options opts = {0};
>> struct strbuf commondir = STRBUF_INIT;
>> struct strbuf gitdir = STRBUF_INIT;
>> + struct repository the_early_repo = {0};
>>
>> opts.respect_includes = 1;
>>
>> if (have_git_dir()) {
>> - opts.commondir = get_git_common_dir();
>> - opts.git_dir = get_git_dir();
>> + opts.repo = the_repository;
>
> I'm not very familiar with the code in setup.c so I apologize for the
> noob question: have_git_dir() returns `startup_info->have_repository
> || the_repository->gitdir`; so is it possible that it returns true but
> the_repository->gitdir is not initialized yet? If so, should we also
> check the_repository->gitdir here (before assigning opts.repo), and
> call BUG() when it is NULL, like get_git_dir() does?
>
> Hmm, nevertheless, I see that you already check `opts.repo &&
> opts.repo->gitdir` before trying to use it in
> do_git_config_sequence(). So it should already cover this case, right?
Right --- the main point is that a BUG() call represents "this can't
happen", or in other words, it's an assertion failure. As a matter of
defensive coding functions like get_git_dir() guard against such cases
to make debugging a little easier and exploitation a little more
difficult when the impossible happens.
[...]
> Thanks a lot for this :) I was thinking of adding it as a preparatory
> patch before the fix itself. May I have your S-o-B as the author?
Sure!
Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
Thanks,
Jonathan
