@matheusbsbs / matheus-bagliano / commits / 8e4710f011

worktree: return allocated string from `get_worktree_git_dir()`

The `get_worktree_git_dir()` function returns a string constant that does not need to be free'd by the caller. This string is computed for three different cases: - If we don't have a worktree we return a path into the Git directory. The returned string is owned by `the_repository`, so there is no need for the caller to free it. - If we have a worktree, but no worktree ID then the caller requests the main worktree. In this case we return a path into the common directory, which again is owned by `the_repository` and thus does not need to be free'd. - In the third case, where we have an actual worktree, we compute the path relative to "$GIT_COMMON_DIR/worktrees/". This string does not need to be released either, even though `git_common_path()` ends up allocating memory. But this doesn't result in a memory leak either because we write into a buffer returned by `get_pathname()`, which returns one out of four static buffers. We're about to drop `git_common_path()` in favor of `repo_common_path()`, which doesn't use the same mechanism but instead returns an allocated string owned by the caller. While we could adapt `get_worktree_git_dir()` to also use `get_pathname()` and print the derived common path into that buffer, the whole schema feels a lot like premature optimization in this context. There are some callsites where we call `get_worktree_git_dir()` in a loop that iterates through all worktrees. But none of these loops seem to be even remotely in the hot path, so saving a single allocation there does not feel worth it. Refactor the function to instead consistently return an allocated path so that we can start using `repo_common_path()` in a subsequent commit. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Feb 7, 2025 at 12:03 UTC 8e4710f011dce286d24838fdafd5ce52cfac5285

8 files changed +40 -15

branch.c

+5 -2

@@ -397,7 +397,7 @@ static void prepare_checked_out_branches(void)

397 worktrees = get_worktrees();

398

399 while (worktrees[i]) {

400 - char *old;

400 + char *old, *wt_gitdir;

401 struct wt_status_state state = { 0 };

402 struct worktree *wt = worktrees[i++];

403 struct string_list update_refs = STRING_LIST_INIT_DUP;

@@ -437,7 +437,8 @@ static void prepare_checked_out_branches(void)

437 }

438 wt_status_state_free_buffers(&state);

439

440 - if (!sequencer_get_update_refs_state(get_worktree_git_dir(wt),

440 + wt_gitdir = get_worktree_git_dir(wt);

441 + if (!sequencer_get_update_refs_state(wt_gitdir,

442 &update_refs)) {

443 struct string_list_item *item;

444 for_each_string_list_item(item, &update_refs) {

@@ -448,6 +449,8 @@ static void prepare_checked_out_branches(void)

449 }

450 string_list_clear(&update_refs, 1);

451 }

452 +

453 + free(wt_gitdir);

454 }

455

456 free_worktrees(worktrees);

builtin/fsck.c

+6 -2

@@ -1057,7 +1057,7 @@ int cmd_fsck(int argc,

1057 struct worktree *wt = *p;

1058 struct index_state istate =

1059 INDEX_STATE_INIT(the_repository);

1060 - char *path;

1060 + char *path, *wt_gitdir;

1061

1062 /*

1063 * Make a copy since the buffer is reusable

@@ -1065,9 +1065,13 @@ int cmd_fsck(int argc,

1065 * while we're examining the index.

1066 */

1067 path = xstrdup(worktree_git_path(the_repository, wt, "index"));

1068 - read_index_from(&istate, path, get_worktree_git_dir(wt));

1068 + wt_gitdir = get_worktree_git_dir(wt);

1069 +

1070 + read_index_from(&istate, path, wt_gitdir);

1071 fsck_index(&istate, path, wt->is_current);

1072 +

1073 discard_index(&istate);

1074 + free(wt_gitdir);

1075 free(path);

1076 }

1077 free_worktrees(worktrees);

builtin/receive-pack.c

+3 -1

@@ -1435,7 +1435,8 @@ static const char *push_to_checkout(unsigned char *hash,

1435

1436 static const char *update_worktree(unsigned char *sha1, const struct worktree *worktree)

1437 {

1438 - const char *retval, *git_dir;

1438 + const char *retval;

1439 + char *git_dir;

1440 struct strvec env = STRVEC_INIT;

1441 int invoked_hook;

1442

@@ -1453,6 +1454,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w

1454 retval = push_to_deploy(sha1, &env, worktree->path);

1455

1456 strvec_clear(&env);

1457 + free(git_dir);

1458 return retval;

1459 }

1460

builtin/worktree.c

+8 -2

@@ -657,8 +657,9 @@ static int can_use_local_refs(const struct add_opts *opts)

657 if (!opts->quiet) {

658 struct strbuf path = STRBUF_INIT;

659 struct strbuf contents = STRBUF_INIT;

660 + char *wt_gitdir = get_worktree_git_dir(NULL);

661

661 - strbuf_add_real_path(&path, get_worktree_git_dir(NULL));

662 + strbuf_add_real_path(&path, wt_gitdir);

663 strbuf_addstr(&path, "/HEAD");

664 strbuf_read_file(&contents, path.buf, 64);

665 strbuf_stripspace(&contents, NULL);

@@ -670,6 +671,7 @@ static int can_use_local_refs(const struct add_opts *opts)

671 path.buf, contents.buf);

672 strbuf_release(&path);

673 strbuf_release(&contents);

674 + free(wt_gitdir);

675 }

676 return 1;

677 }

@@ -1157,6 +1159,9 @@ static void validate_no_submodules(const struct worktree *wt)

1159 struct index_state istate = INDEX_STATE_INIT(the_repository);

1160 struct strbuf path = STRBUF_INIT;

1161 int i, found_submodules = 0;

1162 + char *wt_gitdir;

1163 +

1164 + wt_gitdir = get_worktree_git_dir(wt);

1165

1166 if (is_directory(worktree_git_path(the_repository, wt, "modules"))) {

1167 /*

@@ -1166,7 +1171,7 @@ static void validate_no_submodules(const struct worktree *wt)

1171 */

1172 found_submodules = 1;

1173 } else if (read_index_from(&istate, worktree_git_path(the_repository, wt, "index"),

1169 - get_worktree_git_dir(wt)) > 0) {

1174 + wt_gitdir) > 0) {

1175 for (i = 0; i < istate.cache_nr; i++) {

1176 struct cache_entry *ce = istate.cache[i];

1177 int err;

@@ -1185,6 +1190,7 @@ static void validate_no_submodules(const struct worktree *wt)

1190 }

1191 discard_index(&istate);

1192 strbuf_release(&path);

1193 + free(wt_gitdir);

1194

1195 if (found_submodules)

1196 die(_("working trees containing submodules cannot be moved or removed"));

reachable.c

+5 -1

@@ -65,8 +65,10 @@ static void add_rebase_files(struct rev_info *revs)

65 struct worktree **worktrees = get_worktrees();

67 for (struct worktree **wt = worktrees; *wt; wt++) {

68 + char *wt_gitdir = get_worktree_git_dir(*wt);

69 +

70 strbuf_reset(&buf);

69 - strbuf_addstr(&buf, get_worktree_git_dir(*wt));

71 + strbuf_addstr(&buf, wt_gitdir);

72 strbuf_complete(&buf, '/');

73 len = buf.len;

74 for (size_t i = 0; i < ARRAY_SIZE(path); i++) {

@@ -74,6 +76,8 @@ static void add_rebase_files(struct rev_info *revs)

76 strbuf_addstr(&buf, path[i]);

77 add_one_file(buf.buf, revs);

78 }

79 +

80 + free(wt_gitdir);

81 }

82 strbuf_release(&buf);

83 free_worktrees(worktrees);

revision.c

+6 -1

@@ -1874,15 +1874,20 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)

1874 for (p = worktrees; *p; p++) {

1875 struct worktree *wt = *p;

1876 struct index_state istate = INDEX_STATE_INIT(revs->repo);

1877 + char *wt_gitdir;

1878

1879 if (wt->is_current)

1880 continue; /* current index already taken care of */

1881

1882 + wt_gitdir = get_worktree_git_dir(wt);

1883 +

1884 if (read_index_from(&istate,

1885 worktree_git_path(the_repository, wt, "index"),

1883 - get_worktree_git_dir(wt)) > 0)

1886 + wt_gitdir) > 0)

1887 do_add_index_objects_to_pending(revs, &istate, flags);

1888 +

1889 discard_index(&istate);

1890 + free(wt_gitdir);

1891 }

1892 free_worktrees(worktrees);

1893 }

worktree.c

+6 -5

@@ -59,8 +59,9 @@ static void add_head_info(struct worktree *wt)

59 static int is_current_worktree(struct worktree *wt)

60 {

61 char *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));

62 - const char *wt_git_dir = get_worktree_git_dir(wt);

62 + char *wt_git_dir = get_worktree_git_dir(wt);

63 int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));

64 + free(wt_git_dir);

65 free(git_dir);

66 return is_current;

67 }

@@ -175,14 +176,14 @@ struct worktree **get_worktrees(void)

176 return get_worktrees_internal(0);

177 }

178

178 -const char *get_worktree_git_dir(const struct worktree *wt)

179 +char *get_worktree_git_dir(const struct worktree *wt)

180 {

181 if (!wt)

181 - return repo_get_git_dir(the_repository);

182 + return xstrdup(repo_get_git_dir(the_repository));

183 else if (!wt->id)

183 - return repo_get_common_dir(the_repository);

184 + return xstrdup(repo_get_common_dir(the_repository));

185 else

185 - return git_common_path("worktrees/%s", wt->id);

186 + return xstrdup(git_common_path("worktrees/%s", wt->id));

187 }

188

189 static struct worktree *find_worktree_by_suffix(struct worktree **list,

worktree.h

+1 -1

@@ -39,7 +39,7 @@ int submodule_uses_worktrees(const char *path);

39 * Return git dir of the worktree. Note that the path may be relative.

40 * If wt is NULL, git dir of current worktree is returned.

41 */

42 -const char *get_worktree_git_dir(const struct worktree *wt);

42 +char *get_worktree_git_dir(const struct worktree *wt);

44 /*

45 * Search for the worktree identified unambiguously by `arg` -- typically