Add "const" to the "struct module_clone_data" that we pass to
clone_submodule(), which makes the ownership clear, and stops us from
clobbering the "clone_data->path".
We still need to add to the "reference" member, which is a "struct
string_list". We could do this by having clone_submodule() create its
own, and copy the contents over, but let's instead pass it as a
separate parameter. The main point of doing this is to make it clear
that e.g. "clone_data->path" always comes from the "argv", there's no
ambiguity about whether we can eventually free() the "struct
string_list".
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
builtin/submodule--helper.c | 38 +++++++++++++++++++------------------
1 file changed, 20 insertions(+), 18 deletions(-)
diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
index 29641690c8c..7d5ee6a6261 100644
--- a/builtin/submodule--helper.c
+++ b/builtin/submodule--helper.c
@@ -1650,20 +1650,22 @@ static char *clone_submodule_sm_gitdir(const char *name)
return sm_gitdir;
}
-static int clone_submodule(struct module_clone_data *clone_data)
+static int clone_submodule(const struct module_clone_data *clone_data,
+ struct string_list *reference)
{
char *p;
char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);
char *sm_alternate = NULL, *error_strategy = NULL;
struct child_process cp = CHILD_PROCESS_INIT;
+ const char *clone_data_path;
if (!is_absolute_path(clone_data->path)) {
struct strbuf sb = STRBUF_INIT;
strbuf_addf(&sb, "%s/%s", get_git_work_tree(), clone_data->path);
- clone_data->path = strbuf_detach(&sb, NULL);
+ clone_data_path = strbuf_detach(&sb, NULL);
} else {
- clone_data->path = xstrdup(clone_data->path);
+ clone_data_path = xstrdup(clone_data_path);
}
if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)
@@ -1674,7 +1676,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
if (safe_create_leading_directories_const(sm_gitdir) < 0)
die(_("could not create directory '%s'"), sm_gitdir);
- prepare_possible_alternates(clone_data->name, &clone_data->reference);
+ prepare_possible_alternates(clone_data->name, reference);
strvec_push(&cp.args, "clone");
strvec_push(&cp.args, "--no-checkout");
@@ -1684,9 +1686,9 @@ static int clone_submodule(struct module_clone_data *clone_data)
strvec_push(&cp.args, "--progress");
if (clone_data->depth && *(clone_data->depth))
strvec_pushl(&cp.args, "--depth", clone_data->depth, NULL);
- if (clone_data->reference.nr) {
+ if (reference->nr) {
struct string_list_item *item;
- for_each_string_list_item(item, &clone_data->reference)
+ for_each_string_list_item(item, reference)
strvec_pushl(&cp.args, "--reference",
item->string, NULL);
}
@@ -1705,7 +1707,7 @@ static int clone_submodule(struct module_clone_data *clone_data)
strvec_push(&cp.args, "--");
strvec_push(&cp.args, clone_data->url);
- strvec_push(&cp.args, clone_data->path);
+ strvec_push(&cp.args, clone_data_path);
cp.git_cmd = 1;
prepare_submodule_repo_env(&cp.env);
@@ -1713,25 +1715,25 @@ static int clone_submodule(struct module_clone_data *clone_data)
if(run_command(&cp))
die(_("clone of '%s' into submodule path '%s' failed"),
- clone_data->url, clone_data->path);
+ clone_data->url, clone_data_path);
} else {
struct strbuf sb = STRBUF_INIT;
- if (clone_data->require_init && !access(clone_data->path, X_OK) &&
- !is_empty_dir(clone_data->path))
- die(_("directory not empty: '%s'"), clone_data->path);
- if (safe_create_leading_directories_const(clone_data->path) < 0)
- die(_("could not create directory '%s'"), clone_data->path);
+ if (clone_data->require_init && !access(clone_data_path, X_OK) &&
+ !is_empty_dir(clone_data_path))
+ die(_("directory not empty: '%s'"), clone_data_path);
+ if (safe_create_leading_directories_const(clone_data_path) < 0)
+ die(_("could not create directory '%s'"), clone_data_path);
strbuf_addf(&sb, "%s/index", sm_gitdir);
unlink_or_warn(sb.buf);
strbuf_release(&sb);
}
- connect_work_tree_and_git_dir(clone_data->path, sm_gitdir, 0);
+ connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);
- p = git_pathdup_submodule(clone_data->path, "config");
+ p = git_pathdup_submodule(clone_data_path, "config");
if (!p)
- die(_("could not get submodule directory for '%s'"), clone_data->path);
+ die(_("could not get submodule directory for '%s'"), clone_data_path);
/* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */
git_config_get_string("submodule.alternateLocation", &sm_alternate);
@@ -1810,7 +1812,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)
usage_with_options(git_submodule_helper_usage,
module_clone_options);
- clone_submodule(&clone_data);
+ clone_submodule(&clone_data, &clone_data.reference);
list_objects_filter_release(&filter_options);
return 0;
}
@@ -3088,7 +3090,7 @@ static int add_submodule(const struct add_data *add_data)
if (add_data->depth >= 0)
clone_data.depth = xstrfmt("%d", add_data->depth);
- if (clone_submodule(&clone_data))
+ if (clone_submodule(&clone_data, &clone_data.reference))
return -1;
prepare_submodule_repo_env(&cp.env);
--
2.37.1.1095.g0bd6f54ba8a
