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>

diff --git a/builtin/fetch.c b/builtin/fetch.c
index d5442e1eb7022dc0bfe89e193d513839e5a51649..6196e91798dd075800b24d8b9129f0321f7b7338 100644 (file)
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -1012,7 +1012,7 @@ int cmd_fetch(int argc, const char **argv, const char *prefix)
                struct argv_array options = ARGV_ARRAY_INIT;
 
                add_options_to_argv(&options);
                struct argv_array options = ARGV_ARRAY_INIT;
 
                add_options_to_argv(&options);

-               result = fetch_populated_submodules(options.argc, options.argv,
+               result = fetch_populated_submodules(&options,

                                                    submodule_prefix,
                                                    recurse_submodules,
                                                    verbosity < 0);
                                                    submodule_prefix,
                                                    recurse_submodules,
                                                    verbosity < 0);

diff --git a/submodule.c b/submodule.c
index 784b58039dd078fc1e0c4554820cd0e99c8e41d2..83e157d8d5af8567b93801f0503dcf609d09a1cf 100644 (file)
--- a/submodule.c
+++ b/submodule.c
@@ -586,13 +586,13 @@ static void calculate_changed_submodule_paths(void)
        initialized_fetch_ref_tips = 0;
 }
 
        initialized_fetch_ref_tips = 0;
 }
 

-int fetch_populated_submodules(int num_options, const char **options,
+int fetch_populated_submodules(const struct argv_array *options,

                               const char *prefix, int command_line_option,
                               int quiet)
 {
                               const char *prefix, int command_line_option,
                               int quiet)
 {

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

        struct child_process cp;
        struct child_process cp;

-       const char **argv;
+       struct argv_array argv = ARGV_ARRAY_INIT;

        struct string_list_item *name_for_path;
        const char *work_tree = get_git_work_tree();
        if (!work_tree)
        struct string_list_item *name_for_path;
        const char *work_tree = get_git_work_tree();
        if (!work_tree)


@@ -602,17 +602,13 @@ int fetch_populated_submodules(int num_options, const char **options,
                if (read_cache() < 0)
                        die("index file corrupt");
 
                if (read_cache() < 0)
                        die("index file corrupt");
 

-       /* 6: "fetch" (options) --recurse-submodules-default default "--submodule-prefix" prefix NULL */
-       argv = xcalloc(num_options + 6, sizeof(const char *));
-       argv[argc++] = "fetch";
-       for (i = 0; i < num_options; i++)
-               argv[argc++] = options[i];
-       argv[argc++] = "--recurse-submodules-default";
-       default_argc = argc++;
-       argv[argc++] = "--submodule-prefix";
+       argv_array_push(&argv, "fetch");
+       for (i = 0; i < options->argc; i++)
+               argv_array_push(&argv, options->argv[i]);
+       argv_array_push(&argv, "--recurse-submodules-default");
+       /* default value, "--submodule-prefix" and its value are added later */

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

-       cp.argv = argv;

        cp.env = local_repo_env;
        cp.git_cmd = 1;
        cp.no_stdin = 1;
        cp.env = local_repo_env;
        cp.git_cmd = 1;
        cp.no_stdin = 1;


@@ -672,16 +668,21 @@ int fetch_populated_submodules(int num_options, const char **options,
                        if (!quiet)
                                printf("Fetching submodule %s%s\n", prefix, ce->name);
                        cp.dir = submodule_path.buf;
                        if (!quiet)
                                printf("Fetching submodule %s%s\n", prefix, ce->name);
                        cp.dir = submodule_path.buf;

-                       argv[default_argc] = default_argv;
-                       argv[argc] = submodule_prefix.buf;
+                       argv_array_push(&argv, default_argv);
+                       argv_array_push(&argv, "--submodule-prefix");
+                       argv_array_push(&argv, submodule_prefix.buf);
+                       cp.argv = argv.argv;

                        if (run_command(&cp))
                                result = 1;
                        if (run_command(&cp))
                                result = 1;

+                       argv_array_pop(&argv);
+                       argv_array_pop(&argv);
+                       argv_array_pop(&argv);

                }
                strbuf_release(&submodule_path);
                strbuf_release(&submodule_git_dir);
                strbuf_release(&submodule_prefix);
        }
                }
                strbuf_release(&submodule_path);
                strbuf_release(&submodule_git_dir);
                strbuf_release(&submodule_prefix);
        }

-       free(argv);
+       argv_array_clear(&argv);

 out:
        string_list_clear(&changed_submodule_paths, 1);
        return result;
 out:
        string_list_clear(&changed_submodule_paths, 1);
        return result;

diff --git a/submodule.h b/submodule.h
index e105b0ebe6c06a03af7f82bdfbc9beb66377544f..594b50d51066be35d2f9dc9aa795f3ecdf131573 100644 (file)
--- a/submodule.h
+++ b/submodule.h
@@ -2,6 +2,7 @@
 #define SUBMODULE_H
 
 struct diff_options;
 #define SUBMODULE_H
 
 struct diff_options;

+struct argv_array;

 
 enum {
        RECURSE_SUBMODULES_ON_DEMAND = -1,
 
 enum {
        RECURSE_SUBMODULES_ON_DEMAND = -1,


@@ -23,7 +24,7 @@ void show_submodule_summary(FILE *f, const char *path,
                const char *del, const char *add, const char *reset);
 void set_config_fetch_recurse_submodules(int value);
 void check_for_new_submodule_commits(unsigned char new_sha1[20]);
                const char *del, const char *add, const char *reset);
 void set_config_fetch_recurse_submodules(int value);
 void check_for_new_submodule_commits(unsigned char new_sha1[20]);

-int fetch_populated_submodules(int num_options, const char **options,
+int fetch_populated_submodules(const struct argv_array *options,

                               const char *prefix, int command_line_option,
                               int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);
                               const char *prefix, int command_line_option,
                               int quiet);
 unsigned is_submodule_modified(const char *path, int ignore_untracked);