| Message ID | patch-11.20-ee05254eb6a-20221228T175512Z-avarab@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | leak fixes: various simple leak fixes | expand |
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@gmail.com> > --- > 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 '
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 '
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()". 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". Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> --- builtin/worktree.c | 6 +++--- t/t2401-worktree-prune.sh | 1 + t/t2406-worktree-repair.sh | 1 + 3 files changed, 5 insertions(+), 3 deletions(-)