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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:

> It may be a bit odd, but it automatically means that codepaths that simply 
> don't want to care about the subtle difference between "true" and "empty" 
> will just automatically work, because to them the two cases will look 
> identical, because the strings will compare the same - they have the same 
> data, just different addresses.

I should have mentioned the reason why I did not suggest doing
it this way in my [Janitor] message.

It is not "suttle difference between true and empty".  Empty
means false, and with this approach, it switches the meaning of
valueless form of config to quite the opposite.

In addition to fixing existing breakages, a piece of code that
knew NULL is true and empty is false and coded accordingly, i.e.

	if (!value)
        	Ah we have true;
	else if (!*value)
        	Ok this is false;
	else if (!strcmp(value, "something special")
        	Ok, this is not bool but special;
	else
        	return git_config_bool(var, value);

will now need to be changed to:

	if (value == config_true)
        	Ah we have true;
	else if (!*value)
        	Ok this is false;
	else if (!strcmp(value, "something special")
        	Ok, this is not bool but special;
	else
        	return git_config_bool(var, value);

if you do this.  And the code that was already broken:

	if (!strcmp(value, "somevalue")
		Ok let's use somevalue;
	else if (!strcmp(value, "someothervalue")
		Ok let's use someothervalue;
	else
        	die("oops we do not understand '%s'", value);

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;
	else if (!strcmp(value, "someothervalue")
		Ok let's use someothervalue;
	else
        	die("oops we do not understand '%s'", value);

So it does not buy us anything.  That is why I did not suggest
doing it that way.

> This also means that code (notably the value regexp parsing) that does 
> something like
>
> 	value ? value : ""
>
> just automatically can go away, and just use 'value' directly.

Yes, but that's broken already, isn't it?
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux