Re: Value size limits on git config files

From: Drew Northup <drew.northup@maine.edu>
Date: Apr 5, 2011, 17:35 UTC
Message-ID: <1302024946.17813.57.camel@drew-northup.unet.maine.edu>
In-Reply-To: <BANLkTimLxPdr=mGJfxmdKkeq0zouxz05Uw@mail.gmail.com>

On Tue, 2011-04-05 at 19:01 +0200, Erik Faye-Lund wrote:

> On Tue, Apr 5, 2011 at 6:29 PM, Jeff Adamson <jwa@urbancode.com> wrote:
> I was
> > able to strip enough comments and such from myscript that it then no
> > longer invalidated the config once the value was less than 1024 chars.

Show 14 quoted lines

> It's due to use of a fixed-size buffer. This patch fixes it for me:
> 
> diff --git a/config.c b/config.c
> index 0abcada..bc6ea49 100644
> --- a/config.c
> +++ b/config.c
> @@ -133,23 +133,20 @@ static int get_next_char(void)
> 
>  static char *parse_value(void)
>  {
> -	static char value[1024];
> -	int quote = 0, comment = 0, len = 0, space = 0;
> +	struct strbuf value = STRBUF_INIT;
> +	int quote = 0, comment = 0, space = 0;

Eric, You're doing a lot more here than just making a simple char buffer larger...

Show 10 quoted lines

>  	for (;;) {
>  		int c = get_next_char();
> -		if (len >= sizeof(value) - 1)
> -			return NULL;
>  		if (c == '\n') {
>  			if (quote)
>  				return NULL;
> -			value[len] = 0;
> -			return value;
> +			return strbuf_detach(&value, NULL);

...ditto...

Show 6 quoted lines

>  		}
>  		if (comment)
>  			continue;
>  		if (isspace(c) && !quote) {
> -			if (len)
> +			if (value.len)

...ditto...

Show 9 quoted lines

>  				space++;
>  			continue;
>  		}
> @@ -160,7 +157,7 @@ static char *parse_value(void)
>  			}
>  		}
>  		for (; space; space--)
> -			value[len++] = ' ';
> +			strbuf_addch(&value, ' ');

...ditto...

(The rest cut for discussion...)

The the first question that needs to be asked is: Is there a reason why it was still only 1kiB long? The second is why adopt the struct here and not use an expanded char[] element?

I'm not saying this is wrong by any means, but it is a lot more than just a simple change in the length of a char buffer.

-- 
-Drew Northup
________________________________________________
"As opposed to vegetable or mineral error?"
-John Pescatore, SANS NewsBites Vol. 12 Num. 59