On 2024-03-19 15:32, Junio C Hamano wrote:
Dragan Simic <dsimic@xxxxxxxxxxx> writes:
+ if (use_index && !cached)
+ git_config_get_bool("grep.includeuntracked", &untracked);
Can this ever return an error? E.g.
[grep] includeuntracked = "not really"
After a brief inspection of the code in cache.c, git_config_get_bool()
always returns either 0 or 1, so we should be fine. Thus, any
strangeness in a configuration file would end up not enabling
this option.
If that were the case, then it is not "fine".
When the user triggered an operation which *requires* us to parse
and interpret the meaning of an entry in their configuration file
correctly in order to carry it out, and if that entry has a value
that we do not consider valid, we should notice and complain, before
saying "Nah, this I do not understand, so I'll do one of the two
things I would have done if the value were understandable and would
not tell the user which one I did".
What makes it fine int his case is that git_config_get_bool() dies
when the given value is not a Boolean ;-). The returned value from
the function can be used to tell if the variable does not exist and
the caller should decide to stuff some default value to &untracked
but in this case you do not need to.
I'm sorry for not being meticulous enough in my previous response.
What made me not pay enough attention is that it should all be already
covered properly with the already existing mechanisms for parsing the
git configuration files.
In other words, if invoking git_config_get_bool() over a user's
garbled git configuration file could cause any issues, that would've
been another, pretty much unrelated bug.
