On Wed, Sep 24, 2008 at 02:10:28AM -0400, Deskin Miller <deskinm@umich.edu>=
 wrote:
quoted text
> As the testcase demonstrates, it's possible to have split_cmdline return =
-1 and
quoted text
> deallocate any memory it's allocated, if the config string is missing an =
end
quoted text
> quote.  In both the cases below, the return isn't checked, causing a pret=
ty
quoted text
> immediate segfault.

I think this could go to the commit message.

quoted text
> diff --git a/builtin-merge.c b/builtin-merge.c
> index b280444..dcaf368 100644
> --- a/builtin-merge.c
> +++ b/builtin-merge.c
> @@ -442,6 +442,8 @@ static int git_merge_config(const char *k, const char=
 *v, void *cb)
quoted text
> =20
>  		buf =3D xstrdup(v);
>  		argc =3D split_cmdline(buf, &argv);
> +		if (argc < 0)
> +			die("Bad branch.%s.mergeoptions string", branch);
>  		argv =3D xrealloc(argv, sizeof(*argv) * (argc + 2));
>  		memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
>  		argc++;

ACK.

quoted text
> diff --git a/t/t1300-repo-config.sh b/t/t1300-repo-config.sh
> index 64567fb..3794d23 100755
> --- a/t/t1300-repo-config.sh
> +++ b/t/t1300-repo-config.sh
> @@ -741,4 +741,14 @@ test_expect_success 'symlinked configuration' '
> =20
>  '
> =20
> +test_expect_success 'check split_cmdline return' "
> +	git config alias.split-cmdline-fix 'echo \"' &&
> +	git split-cmdline-fix || test $? =3D 128 &&
> +	echo foo > foo &&
> +	git add foo &&
> +	git commit -m 'initial commit' &&
> +	git config branch.master.mergeoptions 'echo \"' &&
> +	git merge master || test $? =3D 128
> +	"

Why don't you use test_must_fail here?