Tanay Abhra <tanay...@gmail.com> writes:

> On 7/16/2014 9:36 PM, Matthieu Moy wrote:
>> Tanay Abhra <tanay...@gmail.com> writes:
>>> +static void git_config_check_init(void)
>>> +{
>>> +   if (the_config_set.hash_initialized)
>>> +           return;
>>> +   git_configset_init(&the_config_set);
>>> +   git_config(config_set_callback, &the_config_set);
>>> +}
>> So, you're now ignoring the return value of git_config. What is the
>> rationale for this? In particular, why did you reject the "die"
>> possibility (I understood that you were inclined to take this option, so
>> I'm curious why you changed your mind).
> The errors (non accessible, non existent files etc) were already being caught 
> by
> git_config_early(). Since git_config() only returns positive values except
> the weird race case you mentioned, I thought the die confused the reader
> of the patch more than it provided error checking. I also tried myself
> simulating the race condition but failed. All the callers of git_config()
> also ignore the return value, so I ended up ignoring the return value myself.

OK. My preference would be to die, but your argument makes sense.

> But I do think that an access_or_warn() check should be put on git config 
> --file
> and git_configset_add_file since other parts of git follow it. What do
> you think about it, still I will send followup patch correcting the git config
> --file condition where it silently ignores the file access error and 
> continues?

I think it should be done, but should not be your priority (i.e. it's
good to do it, but only if the patch is trivial enough).

Matthieu Moy
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to