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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2008, 00:40 UTC
Message-ID: <7vhcggcokx.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LFD.1.00.0802101538030.2920@woody.linux-foundation.org>

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

> And here's an example of this kind of effect. I'm not actually suggesting 
> you apply this patch, but tell me it isn't simpler done this way?

Yes, that is a good example of simplification.

> So this is where it *does* make a difference whether we use NULL or 
> config_bool, and where config_bool is simply better: it allows a config 
> routine to simply never care..

But that applies only to "originally bool but now has additional states" kind of variables.

For a variable that is never about boolean, if the original code said:

	if (!strcmp(var, "section.variable"))
        	foo = xstrdup(value);

it is wrong (would strdup NULL), and the correct fix would be:

	if (!strcmp(var, "section.variable")) {
		if (!value)
			die("missing value for '%s'", var);
        	foo = xstrdup(value);
	}

It does not make much of a difference if that "if (!value)" becomes "if (value == config_true)". If you omit that check, as your "user.name" example shows, foo may get an empty string or a string "true", neither of which is what the user intended to say.