submodule--helper: fix most "struct pathspec" memory leaks

Call clear_pathspec() at the end of various functions that work with
and allocate a "struct pathspec".

In some cases the zero-initialization here isn't strictly needed, but
as we're moving to a "goto cleanup" pattern let's make sure that it's
safe to call clear_pathspec(), we don't want the data to be
uninitialized.

E.g. for module_foreach() we can see from looking at
module_list_compute() that if it returns non-zero that the "pathspec"
will always have been initialized. But relying on that both assumes
knowledge about parse_pathspec(), and would set up a fragile pattern
going forward.

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>
maint
Ævar Arnfjörð Bjarmason 2022-09-01 01:14:10 +02:00 committed by Junio C Hamano
parent d76260e60a
commit 8fb201d4da
1 changed files with 51 additions and 23 deletions

View File

@ -379,7 +379,7 @@ cleanup:
static int module_foreach(int argc, const char **argv, const char *prefix) static int module_foreach(int argc, const char **argv, const char *prefix)
{ {
struct foreach_cb info = FOREACH_CB_INIT; struct foreach_cb info = FOREACH_CB_INIT;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
struct option module_foreach_options[] = { struct option module_foreach_options[] = {
OPT__QUIET(&info.quiet, N_("suppress output of entering each submodule command")), OPT__QUIET(&info.quiet, N_("suppress output of entering each submodule command")),
@ -391,12 +391,13 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
N_("git submodule foreach [--quiet] [--recursive] [--] <command>"), N_("git submodule foreach [--quiet] [--recursive] [--] <command>"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, module_foreach_options, argc = parse_options(argc, argv, prefix, module_foreach_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);


if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0) if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


info.argc = argc; info.argc = argc;
info.argv = argv; info.argv = argv;
@ -404,7 +405,10 @@ static int module_foreach(int argc, const char **argv, const char *prefix)


for_each_listed_submodule(&list, runcommand_in_submodule_cb, &info); for_each_listed_submodule(&list, runcommand_in_submodule_cb, &info);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


static int starts_with_dot_slash(const char *const path) static int starts_with_dot_slash(const char *const path)
@ -515,7 +519,7 @@ static void init_submodule_cb(const struct cache_entry *list_item, void *cb_data
static int module_init(int argc, const char **argv, const char *prefix) static int module_init(int argc, const char **argv, const char *prefix)
{ {
struct init_cb info = INIT_CB_INIT; struct init_cb info = INIT_CB_INIT;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
int quiet = 0; int quiet = 0;
struct option module_init_options[] = { struct option module_init_options[] = {
@ -526,12 +530,13 @@ static int module_init(int argc, const char **argv, const char *prefix)
N_("git submodule init [<options>] [<path>]"), N_("git submodule init [<options>] [<path>]"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, module_init_options, argc = parse_options(argc, argv, prefix, module_init_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);


if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


/* /*
* If there are no path args and submodule.active is set then, * If there are no path args and submodule.active is set then,
@ -546,7 +551,10 @@ static int module_init(int argc, const char **argv, const char *prefix)


for_each_listed_submodule(&list, init_submodule_cb, &info); for_each_listed_submodule(&list, init_submodule_cb, &info);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


struct status_cb { struct status_cb {
@ -693,7 +701,7 @@ static void status_submodule_cb(const struct cache_entry *list_item,
static int module_status(int argc, const char **argv, const char *prefix) static int module_status(int argc, const char **argv, const char *prefix)
{ {
struct status_cb info = STATUS_CB_INIT; struct status_cb info = STATUS_CB_INIT;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
int quiet = 0; int quiet = 0;
struct option module_status_options[] = { struct option module_status_options[] = {
@ -706,12 +714,13 @@ static int module_status(int argc, const char **argv, const char *prefix)
N_("git submodule status [--quiet] [--cached] [--recursive] [<path>...]"), N_("git submodule status [--quiet] [--cached] [--recursive] [<path>...]"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, module_status_options, argc = parse_options(argc, argv, prefix, module_status_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);


if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


info.prefix = prefix; info.prefix = prefix;
if (quiet) if (quiet)
@ -719,7 +728,10 @@ static int module_status(int argc, const char **argv, const char *prefix)


for_each_listed_submodule(&list, status_submodule_cb, &info); for_each_listed_submodule(&list, status_submodule_cb, &info);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


struct module_cb { struct module_cb {
@ -1258,7 +1270,7 @@ static void sync_submodule_cb(const struct cache_entry *list_item, void *cb_data
static int module_sync(int argc, const char **argv, const char *prefix) static int module_sync(int argc, const char **argv, const char *prefix)
{ {
struct sync_cb info = SYNC_CB_INIT; struct sync_cb info = SYNC_CB_INIT;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
int quiet = 0; int quiet = 0;
int recursive = 0; int recursive = 0;
@ -1272,12 +1284,13 @@ static int module_sync(int argc, const char **argv, const char *prefix)
N_("git submodule sync [--quiet] [--recursive] [<path>]"), N_("git submodule sync [--quiet] [--recursive] [<path>]"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, module_sync_options, argc = parse_options(argc, argv, prefix, module_sync_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);


if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


info.prefix = prefix; info.prefix = prefix;
if (quiet) if (quiet)
@ -1287,7 +1300,10 @@ static int module_sync(int argc, const char **argv, const char *prefix)


for_each_listed_submodule(&list, sync_submodule_cb, &info); for_each_listed_submodule(&list, sync_submodule_cb, &info);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


struct deinit_cb { struct deinit_cb {
@ -1396,7 +1412,7 @@ static void deinit_submodule_cb(const struct cache_entry *list_item,
static int module_deinit(int argc, const char **argv, const char *prefix) static int module_deinit(int argc, const char **argv, const char *prefix)
{ {
struct deinit_cb info = DEINIT_CB_INIT; struct deinit_cb info = DEINIT_CB_INIT;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
int quiet = 0; int quiet = 0;
int force = 0; int force = 0;
@ -1411,6 +1427,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
N_("git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]"), N_("git submodule deinit [--quiet] [-f | --force] [--all | [--] [<path>...]]"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, module_deinit_options, argc = parse_options(argc, argv, prefix, module_deinit_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);
@ -1425,7 +1442,7 @@ static int module_deinit(int argc, const char **argv, const char *prefix)
die(_("Use '--all' if you really want to deinitialize all submodules")); die(_("Use '--all' if you really want to deinitialize all submodules"));


if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


info.prefix = prefix; info.prefix = prefix;
if (quiet) if (quiet)
@ -1435,7 +1452,10 @@ static int module_deinit(int argc, const char **argv, const char *prefix)


for_each_listed_submodule(&list, deinit_submodule_cb, &info); for_each_listed_submodule(&list, deinit_submodule_cb, &info);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


struct module_clone_data { struct module_clone_data {
@ -2540,7 +2560,7 @@ cleanup:


static int module_update(int argc, const char **argv, const char *prefix) static int module_update(int argc, const char **argv, const char *prefix)
{ {
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct update_data opt = UPDATE_DATA_INIT; struct update_data opt = UPDATE_DATA_INIT;
struct list_objects_filter_options filter_options = { 0 }; struct list_objects_filter_options filter_options = { 0 };
int ret; int ret;
@ -2617,8 +2637,8 @@ static int module_update(int argc, const char **argv, const char *prefix)
opt.update_strategy.type = opt.update_default; opt.update_strategy.type = opt.update_default;


if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) { if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
list_objects_filter_release(&filter_options); ret = 1;
return 1; goto cleanup;
} }


if (pathspec.nr) if (pathspec.nr)
@ -2629,8 +2649,10 @@ static int module_update(int argc, const char **argv, const char *prefix)
struct init_cb info = INIT_CB_INIT; struct init_cb info = INIT_CB_INIT;


if (module_list_compute(argc, argv, opt.prefix, if (module_list_compute(argc, argv, opt.prefix,
&pathspec, &list) < 0) &pathspec, &list) < 0) {
return 1; ret = 1;
goto cleanup;
}


/* /*
* If there are no path args and submodule.active is set then, * If there are no path args and submodule.active is set then,
@ -2647,7 +2669,9 @@ static int module_update(int argc, const char **argv, const char *prefix)
} }


ret = update_submodules(&opt); ret = update_submodules(&opt);
cleanup:
list_objects_filter_release(&filter_options); list_objects_filter_release(&filter_options);
clear_pathspec(&pathspec);
return ret; return ret;
} }


@ -2731,7 +2755,7 @@ static int push_check(int argc, const char **argv, const char *prefix)
static int absorb_git_dirs(int argc, const char **argv, const char *prefix) static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
{ {
int i; int i;
struct pathspec pathspec; struct pathspec pathspec = { 0 };
struct module_list list = MODULE_LIST_INIT; struct module_list list = MODULE_LIST_INIT;
unsigned flags = ABSORB_GITDIR_RECURSE_SUBMODULES; unsigned flags = ABSORB_GITDIR_RECURSE_SUBMODULES;
struct option embed_gitdir_options[] = { struct option embed_gitdir_options[] = {
@ -2746,17 +2770,21 @@ static int absorb_git_dirs(int argc, const char **argv, const char *prefix)
N_("git submodule absorbgitdirs [<options>] [<path>...]"), N_("git submodule absorbgitdirs [<options>] [<path>...]"),
NULL NULL
}; };
int ret = 1;


argc = parse_options(argc, argv, prefix, embed_gitdir_options, argc = parse_options(argc, argv, prefix, embed_gitdir_options,
git_submodule_helper_usage, 0); git_submodule_helper_usage, 0);


if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0) if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
return 1; goto cleanup;


for (i = 0; i < list.nr; i++) for (i = 0; i < list.nr; i++)
absorb_git_dir_into_superproject(list.entries[i]->name, flags); absorb_git_dir_into_superproject(list.entries[i]->name, flags);


return 0; ret = 0;
cleanup:
clear_pathspec(&pathspec);
return ret;
} }


static int module_config(int argc, const char **argv, const char *prefix) static int module_config(int argc, const char **argv, const char *prefix)