Re: [PATCH] Fix off-by-one error: don't read the byte before a malloc'd buffer.

From: Junio C Hamano <gitster@pobox.com>
Date: Dec 9, 2007, 18:15 UTC
Message-ID: <7vsl2bpwe1.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <87hcitgpca.fsf@rho.meyering.net>

Jim Meyering <jim@meyering.net> writes:

Show 20 quoted lines

> * config.c (store_write_pair): Don't read value[-1].
>
> Signed-off-by: Jim Meyering <meyering@redhat.com>
> ---
>  config.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/config.c b/config.c
> index ed96213..6031b38 100644
> --- a/config.c
> +++ b/config.c
> @@ -652,7 +652,7 @@ static int store_write_pair(int fd, const char* key, const char* value)
>  	for (i = 0; value[i]; i++)
>  		if (value[i] == ';' || value[i] == '#')
>  			quote = 1;
> -	if (value[i-1] == ' ')
> +	if (i && value[i-1] == ' ')
>  		quote = 1;
>
>  	if (write_in_full(fd, "\t", 1) != 1 ||

Thanks.

The fix may be correct but the comment above the part the patch fixes is very misleading and I wasted a few minutes checking to see what it is doing is correct.

The "quote" variable does not really control quoting; the quoting with backslash is always done for the value, and this check is about adding a dq pair around it, so that the parser does not lose leading or trailing SP or string after start-of-comment marker characters.