Re: Alternative approach to the git config NULL value checking patches..

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 10, 2008, 22:29 UTC
Message-ID: <7vbq6oe98y.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0802101406560.2896@woody.linux-foundation.org>

Linus Torvalds <torvalds@linux-foundation.org> writes:

Show 19 quoted lines

> On Sun, 10 Feb 2008, Junio C Hamano wrote:
> ...
>> will now need to be changed to:
>> 
>> 	if (value == config_true)
>>         	Ah we have true;
>> 	else if (!*value)
>>         	Ok this is false;
>
> And that was done by my patch. 
>
>> still need to be fixed to:
>> 
>> 	if (value == config_true)
>>         	die("oops '%s' is not a bool", var);
>> 	else if (!strcmp(value, "somevalue")
>> 		Ok let's use somevalue;
>
> And this is different from checking against NULL exactly how?

Exactly. My answer to your question is: "It is not different from checking against NULL at all."

The first one (i.e. you needed to do so in your patch) shows that the codepath that is already doing the right thing needs to be modified. If we do not introduce config_true, we do not even have to. You need additional change to correctly functioning codepaths that you should not have to do.

The second one shows that even if we introduce config_true, such an already broken codepath needs to be fixed to check with either NULL or config_true anyway. The need for fix the codepath has not been reduced. Changing the rule does not help for this class of codepath.

But as you seem to imply, it might make sense to equate

	[some-random-section]
        	some-random-variable

to

	[some-random-section]
        	some-random-variable = ""

for variables that cannot possibly have any meaningful "bool" semantics. This third class of variables is a possible benefit your patch brings in. The code can be lax for these variables.

However, it would make things inconsistent ("this variable is bool and the above two forms mean completely opposite things, while that variable is not bool and they mean the same thing"). I am just having a hard time convincing myself that this little detail does not matter.