Skip to content

Commit 8fb201d

Browse files
avargitster
avar
authored and
gitster
committed
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 <[email protected]> Reviewed-by: Glen Choo <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent d76260e commit 8fb201d

File tree

1 file changed

+51
-23
lines changed

1 file changed

+51
-23
lines changed

‎builtin/submodule--helper.c

Lines changed: 51 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -379,7 +379,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
379379
static int module_foreach(int argc, const char **argv, const char *prefix)
380380
{
381381
struct foreach_cb info = FOREACH_CB_INIT;
382-
struct pathspec pathspec;
382+
struct pathspec pathspec = { 0 };
383383
struct module_list list = MODULE_LIST_INIT;
384384
struct option module_foreach_options[] = {
385385
OPT__QUIET(&info.quiet, N_("suppress output of entering each submodule command")),
@@ -391,20 +391,24 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
391391
N_("git submodule foreach [--quiet] [--recursive] [--] <command>"),
392392
NULL
393393
};
394+
int ret = 1;
394395

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

398399
if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
399-
return 1;
400+
goto cleanup;
400401

401402
info.argc = argc;
402403
info.argv = argv;
403404
info.prefix = prefix;
404405

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

407-
return 0;
408+
ret = 0;
409+
cleanup:
410+
clear_pathspec(&pathspec);
411+
return ret;
408412
}
409413

410414
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
515519
static int module_init(int argc, const char **argv, const char *prefix)
516520
{
517521
struct init_cb info = INIT_CB_INIT;
518-
struct pathspec pathspec;
522+
struct pathspec pathspec = { 0 };
519523
struct module_list list = MODULE_LIST_INIT;
520524
int quiet = 0;
521525
struct option module_init_options[] = {
@@ -526,12 +530,13 @@ static int module_init(int argc, const char **argv, const char *prefix)
526530
N_("git submodule init [<options>] [<path>]"),
527531
NULL
528532
};
533+
int ret = 1;
529534

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

533538
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
534-
return 1;
539+
goto cleanup;
535540

536541
/*
537542
* 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)
546551

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

549-
return 0;
554+
ret = 0;
555+
cleanup:
556+
clear_pathspec(&pathspec);
557+
return ret;
550558
}
551559

552560
struct status_cb {
@@ -693,7 +701,7 @@ static void status_submodule_cb(const struct cache_entry *list_item,
693701
static int module_status(int argc, const char **argv, const char *prefix)
694702
{
695703
struct status_cb info = STATUS_CB_INIT;
696-
struct pathspec pathspec;
704+
struct pathspec pathspec = { 0 };
697705
struct module_list list = MODULE_LIST_INIT;
698706
int quiet = 0;
699707
struct option module_status_options[] = {
@@ -706,20 +714,24 @@ static int module_status(int argc, const char **argv, const char *prefix)
706714
N_("git submodule status [--quiet] [--cached] [--recursive] [<path>...]"),
707715
NULL
708716
};
717+
int ret = 1;
709718

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

713722
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
714-
return 1;
723+
goto cleanup;
715724

716725
info.prefix = prefix;
717726
if (quiet)
718727
info.flags |= OPT_QUIET;
719728

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

722-
return 0;
731+
ret = 0;
732+
cleanup:
733+
clear_pathspec(&pathspec);
734+
return ret;
723735
}
724736

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

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

12791292
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
1280-
return 1;
1293+
goto cleanup;
12811294

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

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

1290-
return 0;
1303+
ret = 0;
1304+
cleanup:
1305+
clear_pathspec(&pathspec);
1306+
return ret;
12911307
}
12921308

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

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

14271444
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
1428-
return 1;
1445+
goto cleanup;
14291446

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

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

1438-
return 0;
1455+
ret = 0;
1456+
cleanup:
1457+
clear_pathspec(&pathspec);
1458+
return ret;
14391459
}
14401460

14411461
struct module_clone_data {
@@ -2540,7 +2560,7 @@ static int update_submodules(struct update_data *update_data)
25402560

25412561
static int module_update(int argc, const char **argv, const char *prefix)
25422562
{
2543-
struct pathspec pathspec;
2563+
struct pathspec pathspec = { 0 };
25442564
struct update_data opt = UPDATE_DATA_INIT;
25452565
struct list_objects_filter_options filter_options = { 0 };
25462566
int ret;
@@ -2617,8 +2637,8 @@ static int module_update(int argc, const char **argv, const char *prefix)
26172637
opt.update_strategy.type = opt.update_default;
26182638

26192639
if (module_list_compute(argc, argv, prefix, &pathspec, &opt.list) < 0) {
2620-
list_objects_filter_release(&filter_options);
2621-
return 1;
2640+
ret = 1;
2641+
goto cleanup;
26222642
}
26232643

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

26312651
if (module_list_compute(argc, argv, opt.prefix,
2632-
&pathspec, &list) < 0)
2633-
return 1;
2652+
&pathspec, &list) < 0) {
2653+
ret = 1;
2654+
goto cleanup;
2655+
}
26342656

26352657
/*
26362658
* 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)
26472669
}
26482670

26492671
ret = update_submodules(&opt);
2672+
cleanup:
26502673
list_objects_filter_release(&filter_options);
2674+
clear_pathspec(&pathspec);
26512675
return ret;
26522676
}
26532677

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

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

27532778
if (module_list_compute(argc, argv, prefix, &pathspec, &list) < 0)
2754-
return 1;
2779+
goto cleanup;
27552780

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

2759-
return 0;
2784+
ret = 0;
2785+
cleanup:
2786+
clear_pathspec(&pathspec);
2787+
return ret;
27602788
}
27612789

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

0 commit comments

Comments
 (0)