Re: [PATCH 11/20] worktree: fix a trivial leak in prune_worktrees()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Am 28.12.22 um 19:00 schrieb Ævar Arnfjörð Bjarmason:
> We were leaking both the "struct strbuf" in prune_worktrees(), as well
> as the "path" we got from should_prune_worktree(). Since these were
> the only two uses of the "struct string_list" let's change it to a
> "DUP" and push these to it with "string_list_append_nodup()".

Seems odd at first to only use _nodup() on a _DUP list, but is valid.

> For the string_list_append_nodup() we could also string_list_append()
> the main_path.buf, and then strbuf_release(&main_path) right away. But
> doing it this way avoids an allocation, as we already have the "struct
> strbuf" prepared for appending to "kept".

Similarly we could release the path that should_prune_worktree() gives us
after adding it.

The patch looks good to me.

>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
> ---
>  builtin/worktree.c         | 6 +++---
>  t/t2401-worktree-prune.sh  | 1 +
>  t/t2406-worktree-repair.sh | 1 +
>  3 files changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/builtin/worktree.c b/builtin/worktree.c
> index 591d659faea..865ce9be22b 100644
> --- a/builtin/worktree.c
> +++ b/builtin/worktree.c
> @@ -173,7 +173,7 @@ static void prune_worktrees(void)
>  {
>  	struct strbuf reason = STRBUF_INIT;
>  	struct strbuf main_path = STRBUF_INIT;
> -	struct string_list kept = STRING_LIST_INIT_NODUP;
> +	struct string_list kept = STRING_LIST_INIT_DUP;
>  	DIR *dir = opendir(git_path("worktrees"));
>  	struct dirent *d;
>  	if (!dir)
> @@ -184,14 +184,14 @@ static void prune_worktrees(void)
>  		if (should_prune_worktree(d->d_name, &reason, &path, expire))
>  			prune_worktree(d->d_name, reason.buf);
>  		else if (path)
> -			string_list_append(&kept, path)->util = xstrdup(d->d_name);
> +			string_list_append_nodup(&kept, path)->util = xstrdup(d->d_name);
>  	}
>  	closedir(dir);
>
>  	strbuf_add_absolute_path(&main_path, get_git_common_dir());
>  	/* massage main worktree absolute path to match 'gitdir' content */
>  	strbuf_strip_suffix(&main_path, "/.");
> -	string_list_append(&kept, strbuf_detach(&main_path, NULL));
> +	string_list_append_nodup(&kept, strbuf_detach(&main_path, NULL));
>  	prune_dups(&kept);
>  	string_list_clear(&kept, 1);
>
> diff --git a/t/t2401-worktree-prune.sh b/t/t2401-worktree-prune.sh
> index 3d28c7f06b2..568a47ec426 100755
> --- a/t/t2401-worktree-prune.sh
> +++ b/t/t2401-worktree-prune.sh
> @@ -5,6 +5,7 @@ test_description='prune $GIT_DIR/worktrees'
>  GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME=main
>  export GIT_TEST_DEFAULT_INITIAL_BRANCH_NAME
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success initialize '
> diff --git a/t/t2406-worktree-repair.sh b/t/t2406-worktree-repair.sh
> index 5c44453e1c1..8970780efcc 100755
> --- a/t/t2406-worktree-repair.sh
> +++ b/t/t2406-worktree-repair.sh
> @@ -2,6 +2,7 @@
>
>  test_description='test git worktree repair'
>
> +TEST_PASSES_SANITIZE_LEAK=true
>  . ./test-lib.sh
>
>  test_expect_success setup '




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux