Re: [PATCH 04/10] config: avoid segfault when parsing command-line config

From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Claire Fousse <claire.fousse@ensimag.imag.fr>,
	git@vger.kernel.org, Sylvain Boulme <Sylvain.Boulme@imag.fr>,
	Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 04/10] config: avoid segfault when parsing command-line config
Date: Mon, 13 Jun 2011 10:30:22 -0700	[thread overview]
Message-ID: <7vhb7tod01.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20110609155243.GD25507@sigill.intra.peff.net

Jeff King <peff@peff.net> writes:

> We already check for an empty key on the left side of an
> equals, but we would segfault if there was no content at
> all.
>
> Signed-off-by: Jeff King <peff@peff.net>
> ---
>  config.c               |    2 ++
>  t/t1300-repo-config.sh |    4 ++++
>  2 files changed, 6 insertions(+), 0 deletions(-)
>
> diff --git a/config.c b/config.c
> index ebd404a..2517b62 100644
> --- a/config.c
> +++ b/config.c
> @@ -46,6 +46,8 @@ static int git_config_parse_parameter(const char *text,
>  	struct strbuf **pair;
>  	strbuf_addstr(&tmp, text);
>  	pair = strbuf_split_max(&tmp, '=', 2);
> +	if (!pair[0])
> +		return error("bogus config parameter: %s", text);

This feels wrong.

Asking strbuf_split() to split a string "foo" with "=" delimiter would
give you one element array ("foo", NULL), a string "fo" would give you
("fo", NULL), and a string "f" would give you ("f", NULL).  Shouldn't we
get ("", NULL) if we ask it to split ""?