On Wed, Aug 03 2022, Glen Choo wrote:
> Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes:
>
>> Call clear_pathspec() at the end of various functions that work with
>> and allocate a "struct pathspec".
>>
>> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
>> ---
>> builtin/submodule--helper.c | 74 +++++++++++++++++++++++++------------
>> 1 file changed, 51 insertions(+), 23 deletions(-)
>>
>> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
>> index d958da7dddc..92d32f2877f 100644
>> --- a/builtin/submodule--helper.c
>> +++ b/builtin/submodule--helper.c
>> @@ -367,7 +367,7 @@ static void runcommand_in_submodule_cb(const struct cache_entry *list_item,
>> static int module_foreach(int argc, const char **argv, const char *prefix)
>> {
>> struct foreach_cb info = FOREACH_CB_INIT;
>> - struct pathspec pathspec;
>> + struct pathspec pathspec = { 0 };
>
> Out of curiousity, does this zero-initialization do anything for us
> leaks-wise?
No, because if we leak module_list_compute() must have filled the
pathspec with something it allocated, but...
>> struct module_list list = MODULE_LIST_INIT;
>> struct option module_foreach_options[] = {
>> OPT__QUIET(&info.quiet, N_("suppress output of entering each submodule command")),
>> @@ -379,12 +379,13 @@ static int module_foreach(int argc, const char **argv, const char *prefix)
>> N_("git submodule foreach [--quiet] [--recursive] [--] <command>"),
>> NULL
>> };
>> + int ret = 1;
>>
>> argc = parse_options(argc, argv, prefix, module_foreach_options,
>> git_submodule_helper_usage, 0);
>>
>> if (module_list_compute(0, NULL, prefix, &pathspec, &list) < 0)
>> - return 1;
>> + goto cleanup;
...if we don't initialize it then we can't "goto cleanup".
Now, right now this is redundant, we could just "return 1", as we don't
have a pathspec yet.
But it's generally worth just using the same "cleanup" pattern
everywhere, and not worrying about in your cleanunp code that you only
init'd N/TOTAL variables already.
