Skip to content

Commit 85556d4

Browse files
peffgitster
authored andcommitted
fetch: use argv_array instead of hand-building arrays
Fetch invokes itself recursively when recursing into submodules or handling "fetch --multiple". In both cases, it builds the child's command line by pushing options onto a statically-sized array. In both cases, the array is currently just big enough to handle the largest possible case. However, this technique is brittle and error-prone, so let's replace it with a dynamic argv_array. Signed-off-by: Jeff King <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent ba4d1c7 commit 85556d4

File tree

1 file changed

+25
-22
lines changed

1 file changed

+25
-22
lines changed

builtin/fetch.c

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include "transport.h"
1515
#include "submodule.h"
1616
#include "connected.h"
17+
#include "argv-array.h"
1718

1819
static const char * const builtin_fetch_usage[] = {
1920
"git fetch [<options>] [<repository> [<refspec>...]]",
@@ -841,57 +842,58 @@ static int add_remote_or_group(const char *name, struct string_list *list)
841842
return 1;
842843
}
843844

844-
static void add_options_to_argv(int *argc, const char **argv)
845+
static void add_options_to_argv(struct argv_array *argv)
845846
{
846847
if (dry_run)
847-
argv[(*argc)++] = "--dry-run";
848+
argv_array_push(argv, "--dry-run");
848849
if (prune)
849-
argv[(*argc)++] = "--prune";
850+
argv_array_push(argv, "--prune");
850851
if (update_head_ok)
851-
argv[(*argc)++] = "--update-head-ok";
852+
argv_array_push(argv, "--update-head-ok");
852853
if (force)
853-
argv[(*argc)++] = "--force";
854+
argv_array_push(argv, "--force");
854855
if (keep)
855-
argv[(*argc)++] = "--keep";
856+
argv_array_push(argv, "--keep");
856857
if (recurse_submodules == RECURSE_SUBMODULES_ON)
857-
argv[(*argc)++] = "--recurse-submodules";
858+
argv_array_push(argv, "--recurse-submodules");
858859
else if (recurse_submodules == RECURSE_SUBMODULES_ON_DEMAND)
859-
argv[(*argc)++] = "--recurse-submodules=on-demand";
860+
argv_array_push(argv, "--recurse-submodules=on-demand");
860861
if (verbosity >= 2)
861-
argv[(*argc)++] = "-v";
862+
argv_array_push(argv, "-v");
862863
if (verbosity >= 1)
863-
argv[(*argc)++] = "-v";
864+
argv_array_push(argv, "-v");
864865
else if (verbosity < 0)
865-
argv[(*argc)++] = "-q";
866+
argv_array_push(argv, "-q");
866867

867868
}
868869

869870
static int fetch_multiple(struct string_list *list)
870871
{
871872
int i, result = 0;
872-
const char *argv[12] = { "fetch", "--append" };
873-
int argc = 2;
874-
875-
add_options_to_argv(&argc, argv);
873+
struct argv_array argv = ARGV_ARRAY_INIT;
876874

877875
if (!append && !dry_run) {
878876
int errcode = truncate_fetch_head();
879877
if (errcode)
880878
return errcode;
881879
}
882880

881+
argv_array_pushl(&argv, "fetch", "--append", NULL);
882+
add_options_to_argv(&argv);
883+
883884
for (i = 0; i < list->nr; i++) {
884885
const char *name = list->items[i].string;
885-
argv[argc] = name;
886-
argv[argc + 1] = NULL;
886+
argv_array_push(&argv, name);
887887
if (verbosity >= 0)
888888
printf(_("Fetching %s\n"), name);
889-
if (run_command_v_opt(argv, RUN_GIT_CMD)) {
889+
if (run_command_v_opt(argv.argv, RUN_GIT_CMD)) {
890890
error(_("Could not fetch %s"), name);
891891
result = 1;
892892
}
893+
argv_array_pop(&argv);
893894
}
894895

896+
argv_array_clear(&argv);
895897
return result;
896898
}
897899

@@ -1007,13 +1009,14 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
10071009
}
10081010

10091011
if (!result && (recurse_submodules != RECURSE_SUBMODULES_OFF)) {
1010-
const char *options[10];
1011-
int num_options = 0;
1012-
add_options_to_argv(&num_options, options);
1013-
result = fetch_populated_submodules(num_options, options,
1012+
struct argv_array options = ARGV_ARRAY_INIT;
1013+
1014+
add_options_to_argv(&options);
1015+
result = fetch_populated_submodules(options.argc, options.argv,
10141016
submodule_prefix,
10151017
recurse_submodules,
10161018
verbosity < 0);
1019+
argv_array_clear(&options);
10171020
}
10181021

10191022
/* All names were strdup()ed or strndup()ed */

0 commit comments

Comments
 (0)