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>

diff --git a/branch.c b/branch.c
index 77716966feb918cc55e9cbfa6feca3542644e69e..91297d55ac9f60a155c1165e0b79d3870dc43dd2 100644 (file)
--- a/branch.c
+++ b/branch.c
@@ -397,7 +397,7 @@ static void prepare_checked_out_branches(void)
        worktrees = get_worktrees();
 
        while (worktrees[i]) {
-               char *old;
+               char *old, *wt_gitdir;
                struct wt_status_state state = { 0 };
                struct worktree *wt = worktrees[i++];
                struct string_list update_refs = STRING_LIST_INIT_DUP;
@@ -437,7 +437,8 @@ static void prepare_checked_out_branches(void)
                }
                wt_status_state_free_buffers(&state);
 
-               if (!sequencer_get_update_refs_state(get_worktree_git_dir(wt),
+               wt_gitdir = get_worktree_git_dir(wt);
+               if (!sequencer_get_update_refs_state(wt_gitdir,
                                                     &update_refs)) {
                        struct string_list_item *item;
                        for_each_string_list_item(item, &update_refs) {
@@ -448,6 +449,8 @@ static void prepare_checked_out_branches(void)
                        }
                        string_list_clear(&update_refs, 1);
                }
+
+               free(wt_gitdir);
        }
 
        free_worktrees(worktrees);

diff --git a/builtin/fsck.c b/builtin/fsck.c
index c12203e0121058447cb5ad6158e5598503809db4..eea1d43647f224fbd0557b4f69956753ad5335f8 100644 (file)
--- a/builtin/fsck.c
+++ b/builtin/fsck.c
@@ -1057,7 +1057,7 @@ int cmd_fsck(int argc,
                        struct worktree *wt = *p;
                        struct index_state istate =
                                INDEX_STATE_INIT(the_repository);
-                       char *path;
+                       char *path, *wt_gitdir;
 
                        /*
                         * Make a copy since the buffer is reusable
@@ -1065,9 +1065,13 @@ int cmd_fsck(int argc,
                         * while we're examining the index.
                         */
                        path = xstrdup(worktree_git_path(the_repository, wt, "index"));
-                       read_index_from(&istate, path, get_worktree_git_dir(wt));
+                       wt_gitdir = get_worktree_git_dir(wt);
+
+                       read_index_from(&istate, path, wt_gitdir);
                        fsck_index(&istate, path, wt->is_current);
+
                        discard_index(&istate);
+                       free(wt_gitdir);
                        free(path);
                }
                free_worktrees(worktrees);

diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index b7ea7746094305efdb3dc2d9d15544ce8290906f..d65a441f32d1d4a6230c7439de0f91270dededa6 100644 (file)
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -1435,7 +1435,8 @@ static const char *push_to_checkout(unsigned char *hash,
 
 static const char *update_worktree(unsigned char *sha1, const struct worktree *worktree)
 {
-       const char *retval, *git_dir;
+       const char *retval;
+       char *git_dir;
        struct strvec env = STRVEC_INIT;
        int invoked_hook;
 
@@ -1453,6 +1454,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w
                retval = push_to_deploy(sha1, &env, worktree->path);
 
        strvec_clear(&env);
+       free(git_dir);
        return retval;
 }
 

diff --git a/builtin/worktree.c b/builtin/worktree.c
index 7959b10d267561c7e604d3b1bbb4177764b19d87..2cea9441a6f215237d23a3cbe478687a5398c724 100644 (file)
--- a/builtin/worktree.c
+++ b/builtin/worktree.c
@@ -657,8 +657,9 @@ static int can_use_local_refs(const struct add_opts *opts)
                if (!opts->quiet) {
                        struct strbuf path = STRBUF_INIT;
                        struct strbuf contents = STRBUF_INIT;
+                       char *wt_gitdir = get_worktree_git_dir(NULL);
 
-                       strbuf_add_real_path(&path, get_worktree_git_dir(NULL));
+                       strbuf_add_real_path(&path, wt_gitdir);
                        strbuf_addstr(&path, "/HEAD");
                        strbuf_read_file(&contents, path.buf, 64);
                        strbuf_stripspace(&contents, NULL);
@@ -670,6 +671,7 @@ static int can_use_local_refs(const struct add_opts *opts)
                                  path.buf, contents.buf);
                        strbuf_release(&path);
                        strbuf_release(&contents);
+                       free(wt_gitdir);
                }
                return 1;
        }
@@ -1157,6 +1159,9 @@ static void validate_no_submodules(const struct worktree *wt)
        struct index_state istate = INDEX_STATE_INIT(the_repository);
        struct strbuf path = STRBUF_INIT;
        int i, found_submodules = 0;
+       char *wt_gitdir;
+
+       wt_gitdir = get_worktree_git_dir(wt);
 
        if (is_directory(worktree_git_path(the_repository, wt, "modules"))) {
                /*
@@ -1166,7 +1171,7 @@ static void validate_no_submodules(const struct worktree *wt)
                 */
                found_submodules = 1;
        } else if (read_index_from(&istate, worktree_git_path(the_repository, wt, "index"),
-                                  get_worktree_git_dir(wt)) > 0) {
+                                  wt_gitdir) > 0) {
                for (i = 0; i < istate.cache_nr; i++) {
                        struct cache_entry *ce = istate.cache[i];
                        int err;
@@ -1185,6 +1190,7 @@ static void validate_no_submodules(const struct worktree *wt)
        }
        discard_index(&istate);
        strbuf_release(&path);
+       free(wt_gitdir);
 
        if (found_submodules)
                die(_("working trees containing submodules cannot be moved or removed"));

diff --git a/reachable.c b/reachable.c
index ecf7ccf5041a13a7bd5a2935d53dc678d1749b3a..9ee04c89ec6151b1df64dc7010cfd6b14dd82c3a 100644 (file)
--- a/reachable.c
+++ b/reachable.c
@@ -65,8 +65,10 @@ static void add_rebase_files(struct rev_info *revs)
        struct worktree **worktrees = get_worktrees();
 
        for (struct worktree **wt = worktrees; *wt; wt++) {
+               char *wt_gitdir = get_worktree_git_dir(*wt);
+
                strbuf_reset(&buf);
-               strbuf_addstr(&buf, get_worktree_git_dir(*wt));
+               strbuf_addstr(&buf, wt_gitdir);
                strbuf_complete(&buf, '/');
                len = buf.len;
                for (size_t i = 0; i < ARRAY_SIZE(path); i++) {
@@ -74,6 +76,8 @@ static void add_rebase_files(struct rev_info *revs)
                        strbuf_addstr(&buf, path[i]);
                        add_one_file(buf.buf, revs);
                }
+
+               free(wt_gitdir);
        }
        strbuf_release(&buf);
        free_worktrees(worktrees);

diff --git a/revision.c b/revision.c
index 474fa1e767c8d8f90ce39190ccf8b49cdaa51f54..c4390f0938cbdeb3a08a310cf2ca5aaed944d377 100644 (file)
--- a/revision.c
+++ b/revision.c
@@ -1874,15 +1874,20 @@ void add_index_objects_to_pending(struct rev_info *revs, unsigned int flags)
        for (p = worktrees; *p; p++) {
                struct worktree *wt = *p;
                struct index_state istate = INDEX_STATE_INIT(revs->repo);
+               char *wt_gitdir;
 
                if (wt->is_current)
                        continue; /* current index already taken care of */
 
+               wt_gitdir = get_worktree_git_dir(wt);
+
                if (read_index_from(&istate,
                                    worktree_git_path(the_repository, wt, "index"),
-                                   get_worktree_git_dir(wt)) > 0)
+                                   wt_gitdir) > 0)
                        do_add_index_objects_to_pending(revs, &istate, flags);
+
                discard_index(&istate);
+               free(wt_gitdir);
        }
        free_worktrees(worktrees);
 }

diff --git a/worktree.c b/worktree.c
index 8f4fc10c44b51359b985984239090736d86ed2ee..3b94535963313a2fac08a1fb2a99a081d9726dbd 100644 (file)
--- a/worktree.c
+++ b/worktree.c
@@ -59,8 +59,9 @@ static void add_head_info(struct worktree *wt)
 static int is_current_worktree(struct worktree *wt)
 {
        char *git_dir = absolute_pathdup(repo_get_git_dir(the_repository));
-       const char *wt_git_dir = get_worktree_git_dir(wt);
+       char *wt_git_dir = get_worktree_git_dir(wt);
        int is_current = !fspathcmp(git_dir, absolute_path(wt_git_dir));
+       free(wt_git_dir);
        free(git_dir);
        return is_current;
 }
@@ -175,14 +176,14 @@ struct worktree **get_worktrees(void)
        return get_worktrees_internal(0);
 }
 
-const char *get_worktree_git_dir(const struct worktree *wt)
+char *get_worktree_git_dir(const struct worktree *wt)
 {
        if (!wt)
-               return repo_get_git_dir(the_repository);
+               return xstrdup(repo_get_git_dir(the_repository));
        else if (!wt->id)
-               return repo_get_common_dir(the_repository);
+               return xstrdup(repo_get_common_dir(the_repository));
        else
-               return git_common_path("worktrees/%s", wt->id);
+               return xstrdup(git_common_path("worktrees/%s", wt->id));
 }
 
 static struct worktree *find_worktree_by_suffix(struct worktree **list,

diff --git a/worktree.h b/worktree.h
index 38145df80f41079f301d3aeaaffcc35b7f6760b9..16368588a0d46bdfa2dbba1f8665577ad5147c62 100644 (file)
--- a/worktree.h
+++ b/worktree.h
@@ -39,7 +39,7 @@ int submodule_uses_worktrees(const char *path);
  * Return git dir of the worktree. Note that the path may be relative.
  * If wt is NULL, git dir of current worktree is returned.
  */
-const char *get_worktree_git_dir(const struct worktree *wt);
+char *get_worktree_git_dir(const struct worktree *wt);
 
 /*
  * Search for the worktree identified unambiguously by `arg` -- typically