Re: [PATCH 4/4] add: plug a leak on interactive_add
- From
- Phillip Wood <phillip.wood123@gmail.com>
- Date
- Apr 22, 2024, 15:43 UTC
- Message-ID
- <55f1da17-6f07-4b25-9cf2-0de69bfb5005@gmail.com>
- In-Reply-To
- <a886c086-4b2e-4d41-ad40-b3fca20de42a@gmail.com>
Hi Rubén
On 21/04/2024 11:29, Rubén Justo wrote:
Show 14 quoted lines
> Plug a leak we have since 5a76aff1a6 (add: convert to use > parse_pathspec, 2013-07-14). > > This leak can be triggered with: > $ git add -p anything > > Fixing this leak allows us to mark as leak-free the following tests: > > + t3701-add-interactive.sh > + t7514-commit-patch.sh > > Mark them with "TEST_PASSES_SANITIZE_LEAK=true" to notice and fix > promply any new leak that may be introduced and triggered by them in the > future.
This makes me wonder if we're freeing the pathspec properly when using '--patch' in checkout, reset, restore and stash.
Best Wishes
Phillip
Show 60 quoted lines
> Signed-off-by: Rubén Justo <rjusto@gmail.com>
> ---
> builtin/add.c | 9 ++++++---
> t/t3701-add-interactive.sh | 1 +
> t/t7514-commit-patch.sh | 2 ++
> 3 files changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/add.c b/builtin/add.c
> index ae723bc85e..b7d3ff1e28 100644
> --- a/builtin/add.c
> +++ b/builtin/add.c
> @@ -150,7 +150,7 @@ static int refresh(int verbose, const struct pathspec *pathspec)
> int interactive_add(const char **argv, const char *prefix, int patch)
> {
> struct pathspec pathspec;
> - int unused;
> + int unused, ret;
>
> if (!git_config_get_bool("add.interactive.usebuiltin", &unused))
> warning(_("the add.interactive.useBuiltin setting has been removed!\n"
> @@ -163,9 +163,12 @@ int interactive_add(const char **argv, const char *prefix, int patch)
> prefix, argv);
>
> if (patch)
> - return !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec);
> + ret = !!run_add_p(the_repository, ADD_P_ADD, NULL, &pathspec);
> else
> - return !!run_add_i(the_repository, &pathspec);
> + ret = !!run_add_i(the_repository, &pathspec);
> +
> + clear_pathspec(&pathspec);
> + return ret;
> }
>
> static int edit_patch(int argc, const char **argv, const char *prefix)
> diff --git a/t/t3701-add-interactive.sh b/t/t3701-add-interactive.sh
> index bc55255b0a..04d8333373 100755
> --- a/t/t3701-add-interactive.sh
> +++ b/t/t3701-add-interactive.sh
> @@ -4,6 +4,7 @@ test_description='add -i basic tests'
> GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
> export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
> . "$TEST_DIRECTORY"/lib-terminal.sh
>
> diff --git a/t/t7514-commit-patch.sh b/t/t7514-commit-patch.sh
> index b4de10a5dd..03ba0c0e73 100755
> --- a/t/t7514-commit-patch.sh
> +++ b/t/t7514-commit-patch.sh
> @@ -1,6 +1,8 @@
> #!/bin/sh
>
> test_description='hunk edit with "commit -p -m"'
> +
> +TEST_PASSES_SANITIZE_LEAK=true
> . ./test-lib.sh
>
> test_expect_success 'setup (initial)' '