From 0915a5b4cdf00a8c6c755b77b854725a183993b4 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Fri, 6 Mar 2020 19:03:13 +0000 Subject: [PATCH 1/4] set_git_dir: fix crash when used with real_path() `real_path()` returns result from a shared buffer, inviting subtle reentrance bugs. One of these bugs occur when invoked this way: set_git_dir(real_path(git_dir)) In this case, `real_path()` has reentrance: real_path read_gitfile_gently repo_set_gitdir setup_git_env set_git_dir_1 set_git_dir Later, `set_git_dir()` uses its now-dead parameter: !is_absolute_path(path) Fix this by using a dedicated `strbuf` to hold `strbuf_realpath()`. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- builtin/init-db.c | 4 ++-- cache.h | 2 +- environment.c | 11 ++++++++++- path.c | 2 +- setup.c | 18 +++++++++--------- 5 files changed, 23 insertions(+), 14 deletions(-) diff --git a/builtin/init-db.c b/builtin/init-db.c index 944ec77fe10327..5bf61a7e056623 100644 --- a/builtin/init-db.c +++ b/builtin/init-db.c @@ -356,12 +356,12 @@ int init_db(const char *git_dir, const char *real_git_dir, if (!exist_ok && !stat(real_git_dir, &st)) die(_("%s already exists"), real_git_dir); - set_git_dir(real_path(real_git_dir)); + set_git_dir(real_git_dir, 1); git_dir = get_git_dir(); separate_git_dir(git_dir, original_git_dir); } else { - set_git_dir(real_path(git_dir)); + set_git_dir(git_dir, 1); git_dir = get_git_dir(); } startup_info->have_repository = 1; diff --git a/cache.h b/cache.h index 37c899b53f7c3d..8cee257d3d73a5 100644 --- a/cache.h +++ b/cache.h @@ -543,7 +543,7 @@ const char *get_git_common_dir(void); char *get_object_directory(void); char *get_index_file(void); char *get_graft_file(struct repository *r); -void set_git_dir(const char *path); +void set_git_dir(const char *path, int make_realpath); int get_common_dir_noenv(struct strbuf *sb, const char *gitdir); int get_common_dir(struct strbuf *sb, const char *gitdir); const char *get_git_namespace(void); diff --git a/environment.c b/environment.c index e72a02d0d577da..c436de31eef129 100644 --- a/environment.c +++ b/environment.c @@ -345,11 +345,20 @@ static void update_relative_gitdir(const char *name, free(path); } -void set_git_dir(const char *path) +void set_git_dir(const char *path, int make_realpath) { + struct strbuf realpath = STRBUF_INIT; + + if (make_realpath) { + strbuf_realpath(&realpath, path, 1); + path = realpath.buf; + } + set_git_dir_1(path); if (!is_absolute_path(path)) chdir_notify_register(NULL, update_relative_gitdir, NULL); + + strbuf_release(&realpath); } const char *get_log_output_encoding(void) diff --git a/path.c b/path.c index 88cf59300738a2..c5a8fe4f0c3c12 100644 --- a/path.c +++ b/path.c @@ -850,7 +850,7 @@ const char *enter_repo(const char *path, int strict) } if (is_git_directory(".")) { - set_git_dir("."); + set_git_dir(".", 0); check_repository_format(); return path; } diff --git a/setup.c b/setup.c index 4ea7a0b081bfd2..fa4317e707a0eb 100644 --- a/setup.c +++ b/setup.c @@ -725,7 +725,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, } /* #18, #26 */ - set_git_dir(gitdirenv); + set_git_dir(gitdirenv, 0); free(gitfile); return NULL; } @@ -747,7 +747,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, } else if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) { /* #16d */ - set_git_dir(gitdirenv); + set_git_dir(gitdirenv, 0); free(gitfile); return NULL; } @@ -759,14 +759,14 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, /* both get_git_work_tree() and cwd are already normalized */ if (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */ - set_git_dir(gitdirenv); + set_git_dir(gitdirenv, 0); free(gitfile); return NULL; } offset = dir_inside_of(cwd->buf, worktree); if (offset >= 0) { /* cwd inside worktree? */ - set_git_dir(real_path(gitdirenv)); + set_git_dir(gitdirenv, 1); if (chdir(worktree)) die_errno(_("cannot chdir to '%s'"), worktree); strbuf_addch(cwd, '/'); @@ -775,7 +775,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv, } /* cwd outside worktree */ - set_git_dir(gitdirenv); + set_git_dir(gitdirenv, 0); free(gitfile); return NULL; } @@ -804,7 +804,7 @@ static const char *setup_discovered_git_dir(const char *gitdir, /* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */ if (is_bare_repository_cfg > 0) { - set_git_dir(offset == cwd->len ? gitdir : real_path(gitdir)); + set_git_dir(gitdir, (offset != cwd->len)); if (chdir(cwd->buf)) die_errno(_("cannot come back to cwd")); return NULL; @@ -813,7 +813,7 @@ static const char *setup_discovered_git_dir(const char *gitdir, /* #0, #1, #5, #8, #9, #12, #13 */ set_git_work_tree("."); if (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT)) - set_git_dir(gitdir); + set_git_dir(gitdir, 0); inside_git_dir = 0; inside_work_tree = 1; if (offset >= cwd->len) @@ -856,10 +856,10 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset, die_errno(_("cannot come back to cwd")); root_len = offset_1st_component(cwd->buf); strbuf_setlen(cwd, offset > root_len ? offset : root_len); - set_git_dir(cwd->buf); + set_git_dir(cwd->buf, 0); } else - set_git_dir("."); + set_git_dir(".", 0); return NULL; } From 3d7747e318532a36a263c61cdf92f2decb6424ff Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Tue, 10 Mar 2020 13:11:22 +0000 Subject: [PATCH 2/4] real_path: remove unsafe API Returning a shared buffer invites very subtle bugs due to reentrancy or multi-threading, as demonstrated by the previous patch. There was an unfinished effort to abolish this [1]. Let's finally rid of `real_path()`, using `strbuf_realpath()` instead. This patch uses a local `strbuf` for most places where `real_path()` was previously called. However, two places return the value of `real_path()` to the caller. For them, a `static` local `strbuf` was added, effectively pushing the problem one level higher: read_gitfile_gently() get_superproject_working_tree() [1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/ Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- abspath.c | 8 +------- builtin/clone.c | 6 +++++- builtin/commit-graph.c | 5 ++++- builtin/rev-parse.c | 5 ++++- builtin/worktree.c | 9 ++++++--- cache.h | 1 - editor.c | 11 +++++++++-- environment.c | 7 ++++++- path.c | 2 +- setup.c | 15 ++++++++++++--- submodule.c | 4 +++- t/helper/test-path-utils.c | 5 ++++- worktree.c | 5 ++++- 13 files changed, 59 insertions(+), 24 deletions(-) diff --git a/abspath.c b/abspath.c index 98579853299427..d34026bfeb85b5 100644 --- a/abspath.c +++ b/abspath.c @@ -206,12 +206,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path, * Resolve `path` into an absolute, cleaned-up path. The return value * comes from a shared buffer. */ -const char *real_path(const char *path) -{ - static struct strbuf realpath = STRBUF_INIT; - return strbuf_realpath(&realpath, path, 1); -} - const char *real_path_if_valid(const char *path) { static struct strbuf realpath = STRBUF_INIT; @@ -233,7 +227,7 @@ char *real_pathdup(const char *path, int die_on_error) /* * Use this to get an absolute path from a relative one. If you want - * to resolve links, you should use real_path. + * to resolve links, you should use strbuf_realpath. */ const char *absolute_path(const char *path) { diff --git a/builtin/clone.c b/builtin/clone.c index 1ad26f4d8c81b3..488bdb074172d0 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -420,6 +420,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, struct dir_iterator *iter; int iter_status; unsigned int flags; + struct strbuf realpath = STRBUF_INIT; mkdir_if_missing(dest->buf, 0777); @@ -454,7 +455,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, if (unlink(dest->buf) && errno != ENOENT) die_errno(_("failed to unlink '%s'"), dest->buf); if (!option_no_hardlinks) { - if (!link(real_path(src->buf), dest->buf)) + strbuf_realpath(&realpath, src->buf, 1); + if (!link(realpath.buf, dest->buf)) continue; if (option_local > 0) die_errno(_("failed to create link '%s'"), dest->buf); @@ -468,6 +470,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest, strbuf_setlen(src, src_len); die(_("failed to iterate over '%s'"), src->buf); } + + strbuf_release(&realpath); } static void clone_local(const char *src_repo, const char *dest_repo) diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c index 4a70b33fb5f135..d1ab6625f63203 100644 --- a/builtin/commit-graph.c +++ b/builtin/commit-graph.c @@ -39,14 +39,17 @@ static struct object_directory *find_odb(struct repository *r, { struct object_directory *odb; char *obj_dir_real = real_pathdup(obj_dir, 1); + struct strbuf odb_path_real = STRBUF_INIT; prepare_alt_odb(r); for (odb = r->objects->odb; odb; odb = odb->next) { - if (!strcmp(obj_dir_real, real_path(odb->path))) + strbuf_realpath(&odb_path_real, odb->path, 1); + if (!strcmp(obj_dir_real, odb_path_real.buf)) break; } free(obj_dir_real); + strbuf_release(&odb_path_real); if (!odb) die(_("could not find object directory matching %s"), obj_dir); diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 7a00da820355b6..06ca7175ac7887 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -857,7 +857,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) if (!gitdir && !prefix) gitdir = ".git"; if (gitdir) { - puts(real_path(gitdir)); + struct strbuf realpath = STRBUF_INIT; + strbuf_realpath(&realpath, gitdir, 1); + puts(realpath.buf); + strbuf_release(&realpath); continue; } } diff --git a/builtin/worktree.c b/builtin/worktree.c index 24f22800f38c75..d99db356684fab 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -258,7 +258,7 @@ static int add_worktree(const char *path, const char *refname, const struct add_opts *opts) { struct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT; - struct strbuf sb = STRBUF_INIT; + struct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT; const char *name; struct child_process cp = CHILD_PROCESS_INIT; struct argv_array child_env = ARGV_ARRAY_INIT; @@ -330,9 +330,11 @@ static int add_worktree(const char *path, const char *refname, strbuf_reset(&sb); strbuf_addf(&sb, "%s/gitdir", sb_repo.buf); - write_file(sb.buf, "%s", real_path(sb_git.buf)); + strbuf_realpath(&realpath, sb_git.buf, 1); + write_file(sb.buf, "%s", realpath.buf); + strbuf_realpath(&realpath, get_git_common_dir(), 1); write_file(sb_git.buf, "gitdir: %s/worktrees/%s", - real_path(get_git_common_dir()), name); + realpath.buf, name); /* * This is to keep resolve_ref() happy. We need a valid HEAD * or is_git_directory() will reject the directory. Any value which @@ -418,6 +420,7 @@ static int add_worktree(const char *path, const char *refname, strbuf_release(&sb_repo); strbuf_release(&sb_git); strbuf_release(&sb_name); + strbuf_release(&realpath); return ret; } diff --git a/cache.h b/cache.h index 8cee257d3d73a5..f6937793ec2fde 100644 --- a/cache.h +++ b/cache.h @@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path) int is_directory(const char *); char *strbuf_realpath(struct strbuf *resolved, const char *path, int die_on_error); -const char *real_path(const char *path); const char *real_path_if_valid(const char *path); char *real_pathdup(const char *path, int die_on_error); const char *absolute_path(const char *path); diff --git a/editor.c b/editor.c index f079abbf110268..91989ee8a116aa 100644 --- a/editor.c +++ b/editor.c @@ -54,7 +54,8 @@ static int launch_specified_editor(const char *editor, const char *path, return error("Terminal is dumb, but EDITOR unset"); if (strcmp(editor, ":")) { - const char *args[] = { editor, real_path(path), NULL }; + struct strbuf realpath = STRBUF_INIT; + const char *args[] = { editor, NULL, NULL }; struct child_process p = CHILD_PROCESS_INIT; int ret, sig; int print_waiting_for_editor = advice_waiting_for_editor && isatty(2); @@ -75,16 +76,22 @@ static int launch_specified_editor(const char *editor, const char *path, fflush(stderr); } + strbuf_realpath(&realpath, path, 1); + args[1] = realpath.buf; + p.argv = args; p.env = env; p.use_shell = 1; p.trace2_child_class = "editor"; - if (start_command(&p) < 0) + if (start_command(&p) < 0) { + strbuf_release(&realpath); return error("unable to start editor '%s'", editor); + } sigchain_push(SIGINT, SIG_IGN); sigchain_push(SIGQUIT, SIG_IGN); ret = finish_command(&p); + strbuf_release(&realpath); sig = ret - 128; sigchain_pop(SIGINT); sigchain_pop(SIGQUIT); diff --git a/environment.c b/environment.c index c436de31eef129..10c9061c432cf9 100644 --- a/environment.c +++ b/environment.c @@ -254,8 +254,11 @@ static int git_work_tree_initialized; */ void set_git_work_tree(const char *new_work_tree) { + struct strbuf realpath = STRBUF_INIT; + if (git_work_tree_initialized) { - new_work_tree = real_path(new_work_tree); + strbuf_realpath(&realpath, new_work_tree, 1); + new_work_tree = realpath.buf; if (strcmp(new_work_tree, the_repository->worktree)) die("internal error: work tree has already been set\n" "Current worktree: %s\nNew worktree: %s", @@ -264,6 +267,8 @@ void set_git_work_tree(const char *new_work_tree) } git_work_tree_initialized = 1; repo_set_worktree(the_repository, new_work_tree); + + strbuf_release(&realpath); } const char *get_git_work_tree(void) diff --git a/path.c b/path.c index c5a8fe4f0c3c12..0a42ceb3fb54d3 100644 --- a/path.c +++ b/path.c @@ -723,7 +723,7 @@ static struct passwd *getpw_str(const char *username, size_t len) * then it is a newly allocated string. Returns NULL on getpw failure or * if path is NULL. * - * If real_home is true, real_path($HOME) is used in the expansion. + * If real_home is true, strbuf_realpath($HOME) is used in the expansion. */ char *expand_user_path(const char *path, int real_home) { diff --git a/setup.c b/setup.c index fa4317e707a0eb..1ae3f20301633d 100644 --- a/setup.c +++ b/setup.c @@ -32,6 +32,7 @@ static int abspath_part_inside_repo(char *path) char *path0; int off; const char *work_tree = get_git_work_tree(); + struct strbuf realpath = STRBUF_INIT; if (!work_tree) return -1; @@ -60,8 +61,10 @@ static int abspath_part_inside_repo(char *path) path++; if (*path == '/') { *path = '\0'; - if (fspathcmp(real_path(path0), work_tree) == 0) { + strbuf_realpath(&realpath, path0, 1); + if (fspathcmp(realpath.buf, work_tree) == 0) { memmove(path0, path + 1, len - (path - path0)); + strbuf_release(&realpath); return 0; } *path = '/'; @@ -69,11 +72,14 @@ static int abspath_part_inside_repo(char *path) } /* check whole path */ - if (fspathcmp(real_path(path0), work_tree) == 0) { + strbuf_realpath(&realpath, path0, 1); + if (fspathcmp(realpath.buf, work_tree) == 0) { *path0 = '\0'; + strbuf_release(&realpath); return 0; } + strbuf_release(&realpath); return -1; } @@ -619,6 +625,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code) struct stat st; int fd; ssize_t len; + static struct strbuf realpath = STRBUF_INIT; if (stat(path, &st)) { /* NEEDSWORK: discern between ENOENT vs other errors */ @@ -669,7 +676,9 @@ const char *read_gitfile_gently(const char *path, int *return_error_code) error_code = READ_GITFILE_ERR_NOT_A_REPO; goto cleanup_return; } - path = real_path(dir); + + strbuf_realpath(&realpath, dir, 1); + path = realpath.buf; cleanup_return: if (return_error_code) diff --git a/submodule.c b/submodule.c index 31f391d7d2541c..bad7a788c06da0 100644 --- a/submodule.c +++ b/submodule.c @@ -2170,6 +2170,7 @@ void absorb_git_dir_into_superproject(const char *path, const char *get_superproject_working_tree(void) { + static struct strbuf realpath = STRBUF_INIT; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf sb = STRBUF_INIT; const char *one_up = real_path_if_valid("../"); @@ -2231,7 +2232,8 @@ const char *get_superproject_working_tree(void) super_wt = xstrdup(cwd); super_wt[cwd_len - super_sub_len] = '\0'; - ret = real_path(super_wt); + strbuf_realpath(&realpath, super_wt, 1); + ret = realpath.buf; free(super_wt); } strbuf_release(&sb); diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c index 409034cf4eef59..313a153209c48f 100644 --- a/t/helper/test-path-utils.c +++ b/t/helper/test-path-utils.c @@ -290,11 +290,14 @@ int cmd__path_utils(int argc, const char **argv) } if (argc >= 2 && !strcmp(argv[1], "real_path")) { + struct strbuf realpath = STRBUF_INIT; while (argc > 2) { - puts(real_path(argv[2])); + strbuf_realpath(&realpath, argv[2], 1); + puts(realpath.buf); argc--; argv++; } + strbuf_release(&realpath); return 0; } diff --git a/worktree.c b/worktree.c index eba4fd3a03812f..e7bbf716f6bdc6 100644 --- a/worktree.c +++ b/worktree.c @@ -285,6 +285,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, unsigned flags) { struct strbuf wt_path = STRBUF_INIT; + struct strbuf realpath = STRBUF_INIT; char *path = NULL; int err, ret = -1; @@ -336,7 +337,8 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, goto done; } - ret = fspathcmp(path, real_path(git_common_path("worktrees/%s", wt->id))); + strbuf_realpath(&realpath, git_common_path("worktrees/%s", wt->id), 1); + ret = fspathcmp(path, realpath.buf); if (ret) strbuf_addf_gently(errmsg, _("'%s' does not point back to '%s'"), @@ -344,6 +346,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg, done: free(path); strbuf_release(&wt_path); + strbuf_release(&realpath); return ret; } From 4530a85b4c34f009b5f190eb2dc8367801de5028 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Tue, 10 Mar 2020 13:11:23 +0000 Subject: [PATCH 3/4] real_path_if_valid(): remove unsafe API This commit continues the work started with previous commit. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- abspath.c | 10 ---------- cache.h | 1 - setup.c | 2 +- sha1-file.c | 13 ++++--------- submodule.c | 7 ++++--- worktree.c | 7 +++++-- 6 files changed, 14 insertions(+), 26 deletions(-) diff --git a/abspath.c b/abspath.c index d34026bfeb85b5..6f15a418bb64e2 100644 --- a/abspath.c +++ b/abspath.c @@ -202,16 +202,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path, return retval; } -/* - * Resolve `path` into an absolute, cleaned-up path. The return value - * comes from a shared buffer. - */ -const char *real_path_if_valid(const char *path) -{ - static struct strbuf realpath = STRBUF_INIT; - return strbuf_realpath(&realpath, path, 0); -} - char *real_pathdup(const char *path, int die_on_error) { struct strbuf realpath = STRBUF_INIT; diff --git a/cache.h b/cache.h index f6937793ec2fde..aa3f5ce718a35d 100644 --- a/cache.h +++ b/cache.h @@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path) int is_directory(const char *); char *strbuf_realpath(struct strbuf *resolved, const char *path, int die_on_error); -const char *real_path_if_valid(const char *path); char *real_pathdup(const char *path, int die_on_error); const char *absolute_path(const char *path); char *absolute_pathdup(const char *path); diff --git a/setup.c b/setup.c index 1ae3f20301633d..9e8fa46bc787da 100644 --- a/setup.c +++ b/setup.c @@ -886,7 +886,7 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_ /* * A "string_list_each_func_t" function that canonicalizes an entry - * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or + * from GIT_CEILING_DIRECTORIES using real_pathdup(), or * discards it if unusable. The presence of an empty entry in * GIT_CEILING_DIRECTORIES turns off canonicalization for all * subsequent entries. diff --git a/sha1-file.c b/sha1-file.c index 616886799e5906..f2b24654895329 100644 --- a/sha1-file.c +++ b/sha1-file.c @@ -676,20 +676,15 @@ void add_to_alternates_memory(const char *reference) char *compute_alternate_path(const char *path, struct strbuf *err) { char *ref_git = NULL; - const char *repo, *ref_git_s; + const char *repo; int seen_error = 0; - ref_git_s = real_path_if_valid(path); - if (!ref_git_s) { + ref_git = real_pathdup(path, 0); + if (!ref_git) { seen_error = 1; strbuf_addf(err, _("path '%s' does not exist"), path); goto out; - } else - /* - * Beware: read_gitfile(), real_path() and mkpath() - * return static buffer - */ - ref_git = xstrdup(ref_git_s); + } repo = read_gitfile(ref_git); if (!repo) diff --git a/submodule.c b/submodule.c index bad7a788c06da0..215c62580fc9fb 100644 --- a/submodule.c +++ b/submodule.c @@ -2173,7 +2173,7 @@ const char *get_superproject_working_tree(void) static struct strbuf realpath = STRBUF_INIT; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf sb = STRBUF_INIT; - const char *one_up = real_path_if_valid("../"); + struct strbuf one_up = STRBUF_INIT; const char *cwd = xgetcwd(); const char *ret = NULL; const char *subpath; @@ -2188,10 +2188,11 @@ const char *get_superproject_working_tree(void) */ return NULL; - if (!one_up) + if (!strbuf_realpath(&one_up, "../", 0)) return NULL; - subpath = relative_path(cwd, one_up, &sb); + subpath = relative_path(cwd, one_up.buf, &sb); + strbuf_release(&one_up); prepare_submodule_repo_env(&cp.env_array); argv_array_pop(&cp.env_array); diff --git a/worktree.c b/worktree.c index e7bbf716f6bdc6..543472f0c7bf1b 100644 --- a/worktree.c +++ b/worktree.c @@ -226,17 +226,20 @@ struct worktree *find_worktree(struct worktree **list, struct worktree *find_worktree_by_path(struct worktree **list, const char *p) { + struct strbuf wt_path = STRBUF_INIT; char *path = real_pathdup(p, 0); if (!path) return NULL; for (; *list; list++) { - const char *wt_path = real_path_if_valid((*list)->path); + if (!strbuf_realpath(&wt_path, (*list)->path, 0)) + continue; - if (wt_path && !fspathcmp(path, wt_path)) + if (!fspathcmp(path, wt_path.buf)) break; } free(path); + strbuf_release(&wt_path); return *list; } From 49d3c4b481f12c2ec655a71d5a5b9259a398d059 Mon Sep 17 00:00:00 2001 From: Alexandr Miloslavskiy Date: Tue, 10 Mar 2020 13:11:24 +0000 Subject: [PATCH 4/4] get_superproject_working_tree(): return strbuf Together with the previous commits, this commit fully fixes the problem of using shared buffer for `real_path()` in `get_superproject_working_tree()`. Signed-off-by: Alexandr Miloslavskiy Signed-off-by: Junio C Hamano --- builtin/rev-parse.c | 7 ++++--- submodule.c | 17 ++++++++--------- submodule.h | 4 ++-- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c index 06ca7175ac7887..06056434ed1f55 100644 --- a/builtin/rev-parse.c +++ b/builtin/rev-parse.c @@ -808,9 +808,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix) continue; } if (!strcmp(arg, "--show-superproject-working-tree")) { - const char *superproject = get_superproject_working_tree(); - if (superproject) - puts(superproject); + struct strbuf superproject = STRBUF_INIT; + if (get_superproject_working_tree(&superproject)) + puts(superproject.buf); + strbuf_release(&superproject); continue; } if (!strcmp(arg, "--show-prefix")) { diff --git a/submodule.c b/submodule.c index 215c62580fc9fb..c3aadf3fff8c45 100644 --- a/submodule.c +++ b/submodule.c @@ -2168,14 +2168,13 @@ void absorb_git_dir_into_superproject(const char *path, } } -const char *get_superproject_working_tree(void) +int get_superproject_working_tree(struct strbuf *buf) { - static struct strbuf realpath = STRBUF_INIT; struct child_process cp = CHILD_PROCESS_INIT; struct strbuf sb = STRBUF_INIT; struct strbuf one_up = STRBUF_INIT; const char *cwd = xgetcwd(); - const char *ret = NULL; + int ret = 0; const char *subpath; int code; ssize_t len; @@ -2186,10 +2185,10 @@ const char *get_superproject_working_tree(void) * We might have a superproject, but it is harder * to determine. */ - return NULL; + return 0; if (!strbuf_realpath(&one_up, "../", 0)) - return NULL; + return 0; subpath = relative_path(cwd, one_up.buf, &sb); strbuf_release(&one_up); @@ -2233,8 +2232,8 @@ const char *get_superproject_working_tree(void) super_wt = xstrdup(cwd); super_wt[cwd_len - super_sub_len] = '\0'; - strbuf_realpath(&realpath, super_wt, 1); - ret = realpath.buf; + strbuf_realpath(buf, super_wt, 1); + ret = 1; free(super_wt); } strbuf_release(&sb); @@ -2243,10 +2242,10 @@ const char *get_superproject_working_tree(void) if (code == 128) /* '../' is not a git repository */ - return NULL; + return 0; if (code == 0 && len == 0) /* There is an unrelated git repository at '../' */ - return NULL; + return 0; if (code) die(_("ls-tree returned unexpected return code %d"), code); diff --git a/submodule.h b/submodule.h index c81ec1a9b6c8ff..4dad649f94220e 100644 --- a/submodule.h +++ b/submodule.h @@ -152,8 +152,8 @@ void absorb_git_dir_into_superproject(const char *path, /* * Return the absolute path of the working tree of the superproject, which this * project is a submodule of. If this repository is not a submodule of - * another repository, return NULL. + * another repository, return 0. */ -const char *get_superproject_working_tree(void); +int get_superproject_working_tree(struct strbuf *buf); #endif