Re: [PATCH v2 05/24] submodule--helper: "struct pathspec" memory leak in module_update()
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 20, 2022, 16:43 UTC
- Message-ID
- <xmqqwnc73fba.fsf@gitster.g>
- In-Reply-To
- <patch-v2-05.24-a4672aa9c94-20220719T204458Z-avarab@gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
> The module_update() function calls module_list_compute() twice, which > in turn will reset the "struct pathspec" passed to it. Let's instead > track two of them, and clear them both.
Looks correct. I wish there were an easy way to limit the scope of this new pathspec2 to the block guarded by "if (opt.init)", but with the "jump to error-exit" idiom, there unfortunately isn't.
Thanks.
Show 34 quoted lines
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
> builtin/submodule--helper.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index 28c5fdb8954..7466e781e97 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -2602,6 +2602,7 @@ static int update_submodules(struct update_data *update_data)
> static int module_update(int argc, const char **argv, const char *prefix)
> {
> struct pathspec pathspec = { 0 };
> + struct pathspec pathspec2 = { 0 };
> struct update_data opt = UPDATE_DATA_INIT;
> struct list_objects_filter_options filter_options = { 0 };
> int ret;
> @@ -2692,7 +2693,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
> struct init_cb info = INIT_CB_INIT;
>
> if (module_list_compute(argc, argv, opt.prefix,
> - &pathspec, &list) < 0) {
> + &pathspec2, &list) < 0) {
> ret = 1;
> goto cleanup;
> }
> @@ -2715,6 +2716,7 @@ static int module_update(int argc, const char **argv, const char *prefix)
> cleanup:
> list_objects_filter_release(&filter_options);
> clear_pathspec(&pathspec);
> + clear_pathspec(&pathspec2);
> return ret;
> }