git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH 4/4] add: plug a leak on interactive_add

From
PWPhillip 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)' '
Previous: Rubén JustoNext: Rubén Justo
Message 10 of 19 in “mark t3701-add-interactive.sh as leak-free”
  1. 0/4 mark t3701-add-interactive.sh as leak-freeRubén Justo, Apr 21, 2024
  2. 2/4 add-interactive: plug a leak in get_untracked_filesRubén Justo, Apr 21, 2024
  3. Junio C HamanoApr 22, 2024
  4. 1/4 apply: plug a leak in apply_dataRubén Justo, Apr 21, 2024
  5. Phillip WoodApr 22, 2024
  6. Rubén JustoApr 22, 2024
  7. Junio C HamanoApr 22, 2024
  8. 3/4 add-patch: plug a leak handling the '/' commandRubén Justo, Apr 21, 2024
  9. 4/4 add: plug a leak on interactive_addRubén Justo, Apr 21, 2024
  10. Phillip WoodApr 22, 2024
  11. Rubén JustoApr 22, 2024
  12. Patrick SteinhardtApr 22, 2024
  13. Junio C HamanoApr 22, 2024
  14. 0/4 mark t3701-add-interactive.sh as leak-freeRubén Justo, Apr 22, 2024
  15. 1/4 apply: plug a leak in apply_dataRubén Justo, Apr 22, 2024
  16. 2/4 add-interactive: plug a leak in get_untracked_filesRubén Justo, Apr 22, 2024
  17. 3/4 add-patch: plug a leak handling the '/' commandRubén Justo, Apr 22, 2024
  18. 4/4 add: plug a leak on interactive_addRubén Justo, Apr 22, 2024
  19. Junio C HamanoApr 22, 2024

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.