@matheusbsbs / matheus-bagliano / commits / 50d89ad654

submodule: use argv_array instead of hand-building arrays

fetch_populated_submodules() allocates the full argv array it uses to recurse into the submodules from the number of given options plus the six argv values it is going to add. It then initializes it with those values which won't change during the iteration and copies the given options into it. Inside the loop the two argv values different for each submodule get replaced with those currently valid. However, this technique is brittle and error-prone (as the comment to explain the magic number 6 indicates), so let's replace it with an argv_array. Instead of replacing the argv values, push them to the argv_array just before the run_command() call (including the option separating them) and pop them from the argv_array right after that. Signed-off-by: Jens Lehmann <Jens.Lehmann@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Jens Lehmann committed Sep 1, 2012 at 17:27 UTC 50d89ad6542c8acafefa6d42f8b42dfa9b8fafe1

3 files changed +19 -17

builtin/fetch.c

+1 -1

@@ -1012,7 +1012,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)

1012 struct argv_array options = ARGV_ARRAY_INIT;

1013

1014 add_options_to_argv(&options);

1015 - result = fetch_populated_submodules(options.argc, options.argv,

1015 + result = fetch_populated_submodules(&options,

1016 submodule_prefix,

1017 recurse_submodules,

1018 verbosity < 0);

submodule.c

+16 -15

@@ -586,13 +586,13 @@ static void calculate_changed_submodule_paths(void)

586 initialized_fetch_ref_tips = 0;

587 }

588

589 -int fetch_populated_submodules(int num_options, const char **options,

589 +int fetch_populated_submodules(const struct argv_array *options,

590 const char *prefix, int command_line_option,

591 int quiet)

592 {

593 - int i, result = 0, argc = 0, default_argc;

593 + int i, result = 0;

594 struct child_process cp;

595 - const char **argv;

595 + struct argv_array argv = ARGV_ARRAY_INIT;

596 struct string_list_item *name_for_path;

597 const char *work_tree = get_git_work_tree();

598 if (!work_tree)

@@ -602,17 +602,13 @@ int fetch_populated_submodules(int num_options, const char **options,

602 if (read_cache() < 0)

603 die("index file corrupt");

604

605 - /* 6: "fetch" (options) --recurse-submodules-default default "--submodule-prefix" prefix NULL */

606 - argv = xcalloc(num_options + 6, sizeof(const char *));

607 - argv[argc++] = "fetch";

608 - for (i = 0; i < num_options; i++)

609 - argv[argc++] = options[i];

610 - argv[argc++] = "--recurse-submodules-default";

611 - default_argc = argc++;

612 - argv[argc++] = "--submodule-prefix";

605 + argv_array_push(&argv, "fetch");

606 + for (i = 0; i < options->argc; i++)

607 + argv_array_push(&argv, options->argv[i]);

608 + argv_array_push(&argv, "--recurse-submodules-default");

609 + /* default value, "--submodule-prefix" and its value are added later */

610

611 memset(&cp, 0, sizeof(cp));

615 - cp.argv = argv;

612 cp.env = local_repo_env;

613 cp.git_cmd = 1;

614 cp.no_stdin = 1;

@@ -672,16 +668,21 @@ int fetch_populated_submodules(int num_options, const char **options,

668 if (!quiet)

669 printf("Fetching submodule %s%s\n", prefix, ce->name);

670 cp.dir = submodule_path.buf;

675 - argv[default_argc] = default_argv;

676 - argv[argc] = submodule_prefix.buf;

671 + argv_array_push(&argv, default_argv);

672 + argv_array_push(&argv, "--submodule-prefix");

673 + argv_array_push(&argv, submodule_prefix.buf);

674 + cp.argv = argv.argv;

675 if (run_command(&cp))

676 result = 1;

677 + argv_array_pop(&argv);

678 + argv_array_pop(&argv);

679 + argv_array_pop(&argv);

680 }

681 strbuf_release(&submodule_path);

682 strbuf_release(&submodule_git_dir);

683 strbuf_release(&submodule_prefix);

684 }

684 - free(argv);

685 + argv_array_clear(&argv);

686 out:

687 string_list_clear(&changed_submodule_paths, 1);

688 return result;

submodule.h

+2 -1

@@ -2,6 +2,7 @@

2 #define SUBMODULE_H

4 struct diff_options;

5 +struct argv_array;

7 enum {

8 RECURSE_SUBMODULES_ON_DEMAND = -1,

@@ -23,7 +24,7 @@ void show_submodule_summary(FILE *f, const char *path,

24 const char *del, const char *add, const char *reset);

25 void set_config_fetch_recurse_submodules(int value);

26 void check_for_new_submodule_commits(unsigned char new_sha1[20]);

26 -int fetch_populated_submodules(int num_options, const char **options,

27 +int fetch_populated_submodules(const struct argv_array *options,

28 const char *prefix, int command_line_option,

29 int quiet);

30 unsigned is_submodule_modified(const char *path, int ignore_untracked);