[PATCH] Use strbuf for reading configuration files

[John Keeping]

On Tue, Jun 04, 2013 at 04:47:53PM +0200, Lukas Fleischer wrote:
> Use struct strbuf from Git instead of fixed-size buffers to remove the
> limit on the length of configuration file lines and refactor
> read_config_line() to improve readability.
> 
> Note that this also fixes a buffer overflow that existed with the
> original fixed-size buffer implementation.
> 
> Signed-off-by: Lukas Fleischer <cgit at cryptocrack.de>
> ---

Is there any reason not to use strbuf_getline and
string_list_split_in_place if we're using strbuf here?

>  configfile.c | 65 ++++++++++++++++++++++++++++++------------------------------
>  configfile.h |  2 ++
>  2 files changed, 35 insertions(+), 32 deletions(-)
> 
> diff --git a/configfile.c b/configfile.c
> index d98989c..e6ad1d6 100644
> --- a/configfile.c
> +++ b/configfile.c
> @@ -31,45 +31,44 @@ static void skip_line(FILE *f)
>  		;
>  }
>  
> -static int read_config_line(FILE *f, char *line, const char **value, int bufsize)
> +static int read_config_line(FILE *f, struct strbuf *name, struct strbuf *value)
>  {
> -	int i = 0, isname = 0;
> +	int c = next_char(f);
>  
> -	*value = NULL;
> -	while (i < bufsize - 1) {
> -		int c = next_char(f);
> -		if (!isname && (c == '#' || c == ';')) {
> -			skip_line(f);
> -			continue;
> -		}
> -		if (!isname && isspace(c))
> -			continue;
> +	strbuf_reset(name);
> +	strbuf_reset(value);
>  
> -		if (c == '=' && !*value) {
> -			line[i] = 0;
> -			*value = &line[i + 1];
> -		} else if (c == '\n' && !isname) {
> -			i = 0;
> -			continue;
> -		} else if (c == '\n' || c == EOF) {
> -			line[i] = 0;
> -			break;
> -		} else {
> -			line[i] = c;
> -		}
> -		isname = 1;
> -		i++;
> +	/* Skip comments and preceding spaces. */
> +	while (c == '#' || c == ';') {
> +		skip_line(f);
> +		c = next_char(f);
> +	}
> +	while (isspace(c))
> +		c = next_char(f);
> +
> +	/* Read variable name. */
> +	while (c != '=') {
> +		if (c == '\n' || c == EOF)
> +			return 0;
> +		strbuf_addch(name, c);
> +		c = next_char(f);
>  	}
> -	line[i + 1] = 0;
> -	return i;
> +
> +	/* Read variable value. */
> +	c = next_char(f);
> +	while (c != '\n' && c != EOF) {
> +		strbuf_addch(value, c);
> +		c = next_char(f);
> +	}
> +
> +	return 1;
>  }
>  
>  int parse_configfile(const char *filename, configfile_value_fn fn)
>  {
>  	static int nesting;
> -	int len;
> -	char line[256];
> -	const char *value;
> +	struct strbuf name = STRBUF_INIT;
> +	struct strbuf value = STRBUF_INIT;
>  	FILE *f;
>  
>  	/* cancel deeply nested include-commands */
> @@ -78,10 +77,12 @@ int parse_configfile(const char *filename, configfile_value_fn fn)
>  	if (!(f = fopen(filename, "r")))
>  		return -1;
>  	nesting++;
> -	while ((len = read_config_line(f, line, &value, sizeof(line))) > 0)
> -		fn(line, value);
> +	while (read_config_line(f, &name, &value))
> +		fn(name.buf, value.buf);
>  	nesting--;
>  	fclose(f);
> +	strbuf_release(&name);
> +	strbuf_release(&value);
>  	return 0;
>  }
>  
> diff --git a/configfile.h b/configfile.h
> index 04235e5..af7ca19 100644
> --- a/configfile.h
> +++ b/configfile.h
> @@ -1,6 +1,8 @@
>  #ifndef CONFIGFILE_H
>  #define CONFIGFILE_H
>  
> +#include "cgit.h"
> +
>  typedef void (*configfile_value_fn)(const char *name, const char *value);
>  
>  extern int parse_configfile(const char *filename, configfile_value_fn fn);
> -- 
> 1.8.3.450.gf3f2a46