From: Jeff King <peff@peff.net>
Date: Sat, 01 Sep 2012 11:22:52 GMT
Subject: Re: [PATCH] fetch --all: pass --tags/--no-tags through to each remote
Message-ID: <20120901112251.GA11445@sigill.intra.peff.net>
In-Reply-To: <1346473533-24175-1-git-send-email-ComputerDruid@gmail.com>

On Sat, Sep 01, 2012 at 12:25:33AM -0400, Dan Johnson wrote:

> diff --git a/builtin/fetch.c b/builtin/fetch.c
> index bb9a074..c6bcbdc 100644
> --- a/builtin/fetch.c
> +++ b/builtin/fetch.c
> @@ -857,6 +857,10 @@ static void add_options_to_argv(int *argc, const char **argv)
>  		argv[(*argc)++] = "--recurse-submodules";
>  	else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
>  		argv[(*argc)++] = "--recurse-submodules=on-demand";
> +	if (tags == TAGS_SET)
> +		argv[(*argc)++] = "--tags";
> +	else if (tags == TAGS_UNSET)
> +		argv[(*argc)++] = "--no-tags";
>  	if (verbosity >= 2)
>  		argv[(*argc)++] = "-v";
>  	if (verbosity >= 1)

Hmm. We allocate argv in fetch_multiple like this:

  const char *argv[12] = { "fetch", "--append" };

and then add a bunch of options to it, along with the name of the
remote. By my count, the current code can hit exactly 12 (including the
terminating NULL) if all options are set. Your patch would make it
possible to overflow. Of course, I may be miscounting since it is
extremely error-prone to figure out the right number by tracing each
possible conditional.

Maybe we should switch it to a dynamic argv_array? Like this:

  [1/2]: argv-array: add pop function
  [2/2]: fetch: use argv_array instead of hand-building arrays

-Peff