From: Jeff King <peff@peff.net>
Date: Tue, 01 Jan 2008 06:23:14 GMT
Subject: Re: git config problem with strange config files
Message-ID: <20080101062314.GA21555@coredump.intra.peff.net>
In-Reply-To: <20080101061734.GA15453@coredump.intra.peff.net>

On Tue, Jan 01, 2008 at 01:17:34AM -0500, Jeff King wrote:

> @@ -118,7 +119,7 @@ static int get_value(config_fn_t fn, char *name, unsigned int len)
>  	/* Get the full name */
>  	for (;;) {
>  		c = get_next_char();
> -		if (c == EOF)
> +		if (config_file_eof)
>  			break;
>  		if (!iskeychar(c))
>  			break;
> @@ -182,7 +183,7 @@ static int get_base_var(char *name)
>  
>  	for (;;) {
>  		int c = get_next_char();
> -		if (c == EOF)
> +		if (config_file_eof)
>  			return -1;
>  		if (c == ']')
>  			return baselen;

In these two hunks, we now check the eof flag correctly, which follows
through on the intent of the original code. But if you look carefully at
get_next_char(), it _never_ returns EOF in the first place; it replaces
it with '\n' and sets an external flag. So this 'if' branch was never
reached, but happened to work because the returned newline triggers a
similar effect.

By actually looking at the flag, I am implementing the original intent,
but it is a potential behavior change (I don't _think_ it should matter,
and it passes all of the config tests, but I haven't looked carefully at
all the ramifications). So it might be simpler yet to just eliminate
that conditional return.

-Peff