From: Jeff King <peff@peff.net>
Date: Wed, 03 Aug 2016 16:25:13 GMT
Subject: Re: [PATCHv3 6/7] submodule--helper: add remote-branch helper
Message-ID: <20160803162512.veyff5g52kaqwlgx@sigill.intra.peff.net>
In-Reply-To: <20160729004409.2072-7-sbeller@google.com>

On Thu, Jul 28, 2016 at 05:44:08PM -0700, Stefan Beller wrote:

> +static const char *remote_submodule_branch(const char *path)
> +{
> +	const struct submodule *sub;
> +	gitmodules_config();
> +	git_config(submodule_config, NULL);
> +
> +	sub = submodule_from_path(null_sha1, path);
> +	if (!sub->branch)
> +		return "master";
> +
> +	return sub->branch;
> +}

Coverity complains about "sub" being NULL here, and indeed, it seems
like an easy segfault:

  $ ./git submodule--helper remote-branch foo
  Segmentation fault

I guess this should return NULL in that case. But then this...

> +static int resolve_remote_submodule_branch(int argc, const char **argv,
> +		const char *prefix)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	if (argc != 2)
> +		die("submodule--helper remote-branch takes exactly one arguments, got %d", argc);
> +
> +	printf("%s", remote_submodule_branch(argv[1]));
> +	strbuf_release(&sb);
> +	return 0;
> +}

would need to handle the NULL return, as well. So maybe "master" or the
empty string would be better. I haven't followed the topic closely
enough to have an opinion.

-Peff