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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html