On Wed, Sep 24, 2008 at 02:10:28AM -0400, Deskin Miller <deskinm@xxxxxxxxx> wrote:
> As the testcase demonstrates, it's possible to have split_cmdline return -1 and
> deallocate any memory it's allocated, if the config string is missing an end
> quote. In both the cases below, the return isn't checked, causing a pretty
> immediate segfault.
I think this could go to the commit message.
> 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)
>
> buf = xstrdup(v);
> argc = split_cmdline(buf, &argv);
> + if (argc < 0)
> + die("Bad branch.%s.mergeoptions string", branch);
> argv = xrealloc(argv, sizeof(*argv) * (argc + 2));
> memmove(argv + 1, argv, sizeof(*argv) * (argc + 1));
> argc++;
ACK.
> 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' '
>
> '
>
> +test_expect_success 'check split_cmdline return' "
> + git config alias.split-cmdline-fix 'echo \"' &&
> + git split-cmdline-fix || test \$? = 128 &&
> + echo foo > foo &&
> + git add foo &&
> + git commit -m 'initial commit' &&
> + git config branch.master.mergeoptions 'echo \"' &&
> + git merge master || test \$? = 128
> + "
Why don't you use test_must_fail here?
Attachment:
pgpplaK7qxn9J.pgp
Description: PGP signature
