@matheusbsbs / matheus-bagliano / commits / 6fac5b2f35

submodule--helper: add "const" to passed "module_clone_data"

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". Let's do this by having clone_submodule() create its own, and copy the contents over, allowing us to pass it as a separate parameter. This new "struct string_list" still leaks memory, just as the "struct module_clone_data" did before. let's not fix that for now, to fix that we'll need to add some "goto cleanup" to the relevant code. That will eventually be done in follow-up commits, this change makes it easier to fix the memory leak. The scope of the new "reference" variable in add_submodule() could be narrowed to the "else" block, but as we'll eventually free it with a "goto cleanup" let's declare it at the start of the function. Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Reviewed-by: Glen Choo <chooglen@google.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Sep 1, 2022 at 01:17 UTC 6fac5b2f352efc8c246d6d5be63a66b7b0fc0209

1 file changed +26 -23

builtin/submodule--helper.c

+26 -23

@@ -1434,7 +1434,6 @@ struct module_clone_data {

1434 const char *url;

1435 const char *depth;

1436 struct list_objects_filter_options *filter_options;

1437 - struct string_list reference;

1437 unsigned int quiet: 1;

1438 unsigned int progress: 1;

1439 unsigned int dissociate: 1;

@@ -1442,7 +1441,6 @@ struct module_clone_data {

1441 int single_branch;

1442 };

1443 #define MODULE_CLONE_DATA_INIT { \

1445 - .reference = STRING_LIST_INIT_NODUP, \

1444 .single_branch = -1, \

1445 }

1446

@@ -1569,18 +1567,20 @@ static char *clone_submodule_sm_gitdir(const char *name)

1567 return sm_gitdir;

1568 }

1569

1572 -static int clone_submodule(struct module_clone_data *clone_data)

1570 +static int clone_submodule(const struct module_clone_data *clone_data,

1571 + struct string_list *reference)

1572 {

1573 char *p;

1574 char *sm_gitdir = clone_submodule_sm_gitdir(clone_data->name);

1575 char *sm_alternate = NULL, *error_strategy = NULL;

1576 struct child_process cp = CHILD_PROCESS_INIT;

1577 + const char *clone_data_path;

1578

1579 if (!is_absolute_path(clone_data->path))

1580 - clone_data->path = xstrfmt("%s/%s", get_git_work_tree(),

1581 - clone_data->path);

1580 + clone_data_path = xstrfmt("%s/%s", get_git_work_tree(),

1581 + clone_data->path);

1582 else

1583 - clone_data->path = xstrdup(clone_data->path);

1583 + clone_data_path = xstrdup(clone_data->path);

1584

1585 if (validate_submodule_git_dir(sm_gitdir, clone_data->name) < 0)

1586 die(_("refusing to create/use '%s' in another submodule's "

@@ -1590,7 +1590,7 @@ static int clone_submodule(struct module_clone_data *clone_data)

1590 if (safe_create_leading_directories_const(sm_gitdir) < 0)

1591 die(_("could not create directory '%s'"), sm_gitdir);

1592

1593 - prepare_possible_alternates(clone_data->name, &clone_data->reference);

1593 + prepare_possible_alternates(clone_data->name, reference);

1594

1595 strvec_push(&cp.args, "clone");

1596 strvec_push(&cp.args, "--no-checkout");

@@ -1600,10 +1600,10 @@ static int clone_submodule(struct module_clone_data *clone_data)

1600 strvec_push(&cp.args, "--progress");

1601 if (clone_data->depth && *(clone_data->depth))

1602 strvec_pushl(&cp.args, "--depth", clone_data->depth, NULL);

1603 - if (clone_data->reference.nr) {

1603 + if (reference->nr) {

1604 struct string_list_item *item;

1605

1606 - for_each_string_list_item(item, &clone_data->reference)

1606 + for_each_string_list_item(item, reference)

1607 strvec_pushl(&cp.args, "--reference",

1608 item->string, NULL);

1609 }

@@ -1622,7 +1622,7 @@ static int clone_submodule(struct module_clone_data *clone_data)

1622

1623 strvec_push(&cp.args, "--");

1624 strvec_push(&cp.args, clone_data->url);

1625 - strvec_push(&cp.args, clone_data->path);

1625 + strvec_push(&cp.args, clone_data_path);

1626

1627 cp.git_cmd = 1;

1628 prepare_submodule_repo_env(&cp.env);

@@ -1630,25 +1630,25 @@ static int clone_submodule(struct module_clone_data *clone_data)

1630

1631 if(run_command(&cp))

1632 die(_("clone of '%s' into submodule path '%s' failed"),

1633 - clone_data->url, clone_data->path);

1633 + clone_data->url, clone_data_path);

1634 } else {

1635 char *path;

1636

1637 - if (clone_data->require_init && !access(clone_data->path, X_OK) &&

1638 - !is_empty_dir(clone_data->path))

1639 - die(_("directory not empty: '%s'"), clone_data->path);

1640 - if (safe_create_leading_directories_const(clone_data->path) < 0)

1641 - die(_("could not create directory '%s'"), clone_data->path);

1637 + if (clone_data->require_init && !access(clone_data_path, X_OK) &&

1638 + !is_empty_dir(clone_data_path))

1639 + die(_("directory not empty: '%s'"), clone_data_path);

1640 + if (safe_create_leading_directories_const(clone_data_path) < 0)

1641 + die(_("could not create directory '%s'"), clone_data_path);

1642 path = xstrfmt("%s/index", sm_gitdir);

1643 unlink_or_warn(path);

1644 free(path);

1645 }

1646

1647 - connect_work_tree_and_git_dir(clone_data->path, sm_gitdir, 0);

1647 + connect_work_tree_and_git_dir(clone_data_path, sm_gitdir, 0);

1648

1649 - p = git_pathdup_submodule(clone_data->path, "config");

1649 + p = git_pathdup_submodule(clone_data_path, "config");

1650 if (!p)

1651 - die(_("could not get submodule directory for '%s'"), clone_data->path);

1651 + die(_("could not get submodule directory for '%s'"), clone_data_path);

1652

1653 /* setup alternateLocation and alternateErrorStrategy in the cloned submodule if needed */

1654 git_config_get_string("submodule.alternateLocation", &sm_alternate);

@@ -1673,6 +1673,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)

1673 int dissociate = 0, quiet = 0, progress = 0, require_init = 0;

1674 struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;

1675 struct list_objects_filter_options filter_options = { 0 };

1676 + struct string_list reference = STRING_LIST_INIT_NODUP;

1677 struct option module_clone_options[] = {

1678 OPT_STRING(0, "prefix", &clone_data.prefix,

1679 N_("path"),

@@ -1686,7 +1687,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)

1687 OPT_STRING(0, "url", &clone_data.url,

1688 N_("string"),

1689 N_("url where to clone the submodule from")),

1689 - OPT_STRING_LIST(0, "reference", &clone_data.reference,

1690 + OPT_STRING_LIST(0, "reference", &reference,

1691 N_("repo"),

1692 N_("reference repository")),

1693 OPT_BOOL(0, "dissociate", &dissociate,

@@ -1725,7 +1726,7 @@ static int module_clone(int argc, const char **argv, const char *prefix)

1726 usage_with_options(git_submodule_helper_usage,

1727 module_clone_options);

1728

1728 - clone_submodule(&clone_data);

1729 + clone_submodule(&clone_data, &reference);

1730 list_objects_filter_release(&filter_options);

1731 return 0;

1732 }

@@ -2913,6 +2914,7 @@ static int add_submodule(const struct add_data *add_data)

2914 {

2915 char *submod_gitdir_path;

2916 struct module_clone_data clone_data = MODULE_CLONE_DATA_INIT;

2917 + struct string_list reference = STRING_LIST_INIT_NODUP;

2918

2919 /* perhaps the path already exists and is already a git repo, else clone it */

2920 if (is_directory(add_data->sm_path)) {

@@ -2929,6 +2931,7 @@ static int add_submodule(const struct add_data *add_data)

2931 free(submod_gitdir_path);

2932 } else {

2933 struct child_process cp = CHILD_PROCESS_INIT;

2934 +

2935 submod_gitdir_path = xstrfmt(".git/modules/%s", add_data->sm_name);

2936

2937 if (is_directory(submod_gitdir_path)) {

@@ -2968,13 +2971,13 @@ static int add_submodule(const struct add_data *add_data)

2971 clone_data.quiet = add_data->quiet;

2972 clone_data.progress = add_data->progress;

2973 if (add_data->reference_path)

2971 - string_list_append(&clone_data.reference,

2974 + string_list_append(&reference,

2975 xstrdup(add_data->reference_path));

2976 clone_data.dissociate = add_data->dissociate;

2977 if (add_data->depth >= 0)

2978 clone_data.depth = xstrfmt("%d", add_data->depth);

2979

2977 - if (clone_submodule(&clone_data))

2980 + if (clone_submodule(&clone_data, &reference))

2981 return -1;

2982

2983 prepare_submodule_repo_env(&cp.env);