path: refactor `repo_common_path()` family of functions

The functions provided by the "path" subsystem to derive repository
paths for the commondir, gitdir, worktrees and submodules are quite
inconsistent. Some functions have a `strbuf_` prefix, others have
different return values, some don't provide a variant working on top of
`strbuf`s.

We're thus about to refactor all of these family of functions so that
they follow a common pattern:

  - `repo_*_path()` returns an allocated string.

  - `repo_*_path_append()` appends the path to the caller-provided
    buffer while returning a constant pointer to the buffer. This
    clarifies whether the buffer is being appended to or rewritten,
    which otherwise wasn't immediately obvious.

  - `repo_*_path_replace()` replaces contents of the buffer with the
    computed path, again returning a pointer to the buffer contents.

The returned constant pointer isn't being used anywhere yet, but it will
be used in subsequent commits. Its intent is to allow calling patterns
like the following somewhat contrived example:

    if (!stat(&st, repo_common_path_replace(repo, &buf, ...)) &&
        !unlink(repo_common_path_replace(repo, &buf, ...)))
            ...

Refactor the commondir family of functions accordingly and adapt all
callers.

Note that `repo_common_pathv()` is converted into an internal
implementation detail. It is only used to implement `the_repository`
compatibility shims and will eventually be removed from the public
interface.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/loose.c b/loose.c
index 897ba389daa18cbb72ca8ce2ecbeb77ecdad1a55..51ef490f935adc8f664bdee047cf15e5ca86082b 100644 (file)
--- a/loose.c
+++ b/loose.c
@@ -75,7 +75,7 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire
        insert_loose_map(dir, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
        insert_loose_map(dir, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
 
-       strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+       repo_common_path_replace(repo, &path, "objects/loose-object-idx");
        fp = fopen(path.buf, "rb");
        if (!fp) {
                strbuf_release(&path);
@@ -133,7 +133,7 @@ int repo_write_loose_object_map(struct repository *repo)
        if (!should_use_loose_object_map(repo))
                return 0;
 
-       strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+       repo_common_path_replace(repo, &path, "objects/loose-object-idx");
        fd = hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
        iter = kh_begin(map);
        if (write_in_full(fd, loose_object_header, strlen(loose_object_header)) < 0)
@@ -174,7 +174,7 @@ static int write_one_object(struct repository *repo, const struct object_id *oid
        struct stat st;
        struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
 
-       strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
+       repo_common_path_replace(repo, &path, "objects/loose-object-idx");
        hold_lock_file_for_update_timeout(&lock, path.buf, LOCK_DIE_ON_ERROR, -1);
 
        fd = open(path.buf, O_WRONLY | O_CREAT | O_APPEND, 0666);

diff --git a/path.c b/path.c
index 07964f5d32c97711f39e65c8b0956b0f274508f6..273b649e00c2d7c9877227773762326a084407b1 100644 (file)
--- a/path.c
+++ b/path.c
@@ -414,7 +414,7 @@ static void strbuf_worktree_gitdir(struct strbuf *buf,
        else if (!wt->id)
                strbuf_addstr(buf, repo->commondir);
        else
-               strbuf_git_common_path(buf, repo, "worktrees/%s", wt->id);
+               repo_common_path_append(repo, buf, "worktrees/%s", wt->id);
 }
 
 void repo_git_pathv(const struct repository *repo,
@@ -596,14 +596,38 @@ void repo_common_pathv(const struct repository *repo,
        strbuf_cleanup_path(sb);
 }
 
-void strbuf_git_common_path(struct strbuf *sb,
-                           const struct repository *repo,
-                           const char *fmt, ...)
+char *repo_common_path(const struct repository *repo,
+                      const char *fmt, ...)
+{
+       struct strbuf sb = STRBUF_INIT;
+       va_list args;
+       va_start(args, fmt);
+       repo_common_pathv(repo, &sb, fmt, args);
+       va_end(args);
+       return strbuf_detach(&sb, NULL);
+}
+
+const char *repo_common_path_append(const struct repository *repo,
+                                   struct strbuf *sb,
+                                   const char *fmt, ...)
+{
+       va_list args;
+       va_start(args, fmt);
+       repo_common_pathv(repo, sb, fmt, args);
+       va_end(args);
+       return sb->buf;
+}
+
+const char *repo_common_path_replace(const struct repository *repo,
+                                    struct strbuf *sb,
+                                    const char *fmt, ...)
 {
        va_list args;
+       strbuf_reset(sb);
        va_start(args, fmt);
        repo_common_pathv(repo, sb, fmt, args);
        va_end(args);
+       return sb->buf;
 }
 
 static struct passwd *getpw_str(const char *username, size_t len)

diff --git a/path.h b/path.h
index 5f6c85e5f8d795f14a20c008cbb47578a870292f..3c75495e1a55e7ce4e3be290c77ae080d872ff39 100644 (file)
--- a/path.h
+++ b/path.h
@@ -25,22 +25,20 @@ char *mkpathdup(const char *fmt, ...)
        __attribute__((format (printf, 1, 2)));
 
 /*
- * The `strbuf_git_common_path` family of functions will construct a path into a
+ * The `repo_common_path` family of functions will construct a path into a
  * repository's common git directory, which is shared by all worktrees.
  */
-
-/*
- * Constructs a path into the common git directory of repository `repo` and
- * append it in the provided buffer `sb`.
- */
-void strbuf_git_common_path(struct strbuf *sb,
-                           const struct repository *repo,
-                           const char *fmt, ...)
+char *repo_common_path(const struct repository *repo,
+                      const char *fmt, ...)
+       __attribute__((format (printf, 2, 3)));
+const char *repo_common_path_append(const struct repository *repo,
+                                   struct strbuf *sb,
+                                   const char *fmt, ...)
+       __attribute__((format (printf, 3, 4)));
+const char *repo_common_path_replace(const struct repository *repo,
+                                    struct strbuf *sb,
+                                    const char *fmt, ...)
        __attribute__((format (printf, 3, 4)));
-void repo_common_pathv(const struct repository *repo,
-                      struct strbuf *buf,
-                      const char *fmt,
-                      va_list args);
 
 /*
  * The `repo_git_path` family of functions will construct a path into a repository's
@@ -243,6 +241,12 @@ struct strbuf *get_pathname(void);
 #  include "strbuf.h"
 #  include "repository.h"
 
+/* Internal implementation detail that should not be used. */
+void repo_common_pathv(const struct repository *repo,
+                      struct strbuf *buf,
+                      const char *fmt,
+                      va_list args);
+
 /*
  * Return a statically allocated path into the main repository's
  * (the_repository) common git directory.

diff --git a/refs.c b/refs.c
index f4094a326a9f88f979654b668cc9c3d27d83cb5d..daf6a842050a1630a6a5a21459263848a3f38732 100644 (file)
--- a/refs.c
+++ b/refs.c
@@ -2184,8 +2184,8 @@ struct ref_store *get_worktree_ref_store(const struct worktree *wt)
 
        if (wt->id) {
                struct strbuf common_path = STRBUF_INIT;
-               strbuf_git_common_path(&common_path, wt->repo,
-                                     "worktrees/%s", wt->id);
+               repo_common_path_append(wt->repo, &common_path,
+                                       "worktrees/%s", wt->id);
                refs = ref_store_init(wt->repo, wt->repo->ref_storage_format,
                                      common_path.buf, REF_STORE_ALL_CAPS);
                strbuf_release(&common_path);

diff --git a/setup.c b/setup.c
index 8a488f3e7c74b1ffeef8d25ffb656b990a4c06a6..74b5ba532555e1c645ff695d0a85e96aef66ef48 100644 (file)
--- a/setup.c
+++ b/setup.c
@@ -792,7 +792,7 @@ int upgrade_repository_format(int target_version)
        struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
        int ret;
 
-       strbuf_git_common_path(&sb, the_repository, "config");
+       repo_common_path_append(the_repository, &sb, "config");
        read_repository_format(&repo_fmt, sb.buf);
        strbuf_release(&sb);
 
@@ -2242,7 +2242,7 @@ void initialize_repository_version(int hash_algo,
                struct strbuf config = STRBUF_INIT;
                struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
 
-               strbuf_git_common_path(&config, the_repository, "config");
+               repo_common_path_append(the_repository, &config, "config");
                read_repository_format(&repo_fmt, config.buf);
 
                if (repo_fmt.v1_only_extensions.nr)

diff --git a/worktree.c b/worktree.c
index 248bbb39d43ae82ad843d4a00632cc2722454ea5..f8d6e7127f4ab8b6504a7c3a2c82d5b117740d51 100644 (file)
--- a/worktree.c
+++ b/worktree.c
@@ -104,7 +104,7 @@ struct worktree *get_linked_worktree(const char *id,
        if (!id)
                die("Missing linked worktree name");
 
-       strbuf_git_common_path(&path, the_repository, "worktrees/%s/gitdir", id);
+       repo_common_path_append(the_repository, &path, "worktrees/%s/gitdir", id);
        if (strbuf_read_file(&worktree_path, path.buf, 0) <= 0)
                /* invalid gitdir file */
                goto done;
@@ -731,8 +731,7 @@ static ssize_t infer_backlink(const char *gitfile, struct strbuf *inferred)
        id++; /* advance past '/' to point at <id> */
        if (!*id)
                goto error;
-       strbuf_reset(inferred);
-       strbuf_git_common_path(inferred, the_repository, "worktrees/%s", id);
+       repo_common_path_replace(the_repository, inferred, "worktrees/%s", id);
        if (!is_directory(inferred->buf))
                goto error;