{"thread":{"id":"52949","subject":"[PATCH 0/4] Fix bugs related to real_path()","startedAt":"2020-03-06T19:03:21Z","lastAt":"2020-03-10T13:18:01Z","messageCount":17,"participants":["Alexandr Miloslavskiy via GitGitGadget","Junio C Hamano","Alexandr Miloslavskiy"],"isPatch":true,"patchVersion":1,"patchTotal":4},"messages":[{"id":"392918","messageId":"pull.575.git.1583521396.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":null,"subject":"[PATCH 0/4] Fix bugs related to real_path()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-06T19:03:12Z","receivedAt":"2020-03-06T19:03:21Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"The issue with `real_path()` seems to be long-standing, where multiple\npeople solved parts of it over time. I'm adding another part here\nafter I have discovered a crash related to it.\n\nEven with this step, there are still problems remaining:\n* `read_gitfile_gently()` still uses shared buffer.\n* `absolute_path()` was not removed.\n\nThese issues remain because there're too many code references and I'd like\nto avoid submitting a single topic of a scary size.\n\nAlexandr Miloslavskiy (4):\n set_git_dir: fix crash when used with real_path()\n real_path: remove unsafe API\n real_path_if_valid(): remove unsafe API\n get_superproject_working_tree(): return strbuf\n\n abspath.c | 18 +-----------------\n builtin/clone.c | 7 ++++++-\n builtin/commit-graph.c | 6 +++++-\n builtin/init-db.c | 4 ++--\n builtin/rev-parse.c | 12 ++++++++----\n builtin/worktree.c | 10 +++++++---\n cache.h | 4 +---\n editor.c | 11 +++++++++--\n environment.c | 18 ++++++++++++++++--\n path.c | 4 ++--\n setup.c | 37 ++++++++++++++++++++++++-------------\n sha1-file.c | 13 ++++---------\n submodule.c | 22 ++++++++++++----------\n submodule.h | 4 ++--\n t/helper/test-path-utils.c | 6 +++++-\n worktree.c | 13 ++++++++++---\n 16 files changed, 114 insertions(+), 75 deletions(-)\n\n\nbase-commit: 076cbdcd739aeb33c1be87b73aebae5e43d7bcc5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-575%2FSyntevoAlex%2F%230205(git)_crash_real_path-v1\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-575/SyntevoAlex/#0205(git)_crash_real_path-v1\nPull-Request: https://github.com/gitgitgadget/git/pull/575\n-- \ngitgitgadget\n"},{"id":"392919","messageId":"f7afcb4cc83a955b04283475facc02349207557c.1583521396.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.git.1583521396.gitgitgadget@gmail.com","subject":"[PATCH 1/4] set_git_dir: fix crash when used with real_path()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-06T19:03:13Z","receivedAt":"2020-03-06T19:03:22Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`real_path()` returns result from a shared buffer, inviting subtle\nreentrance bugs. One of these bugs occur when invoked this way:\n set_git_dir(real_path(git_dir))\n\nIn this case, `real_path()` has reentrance:\n real_path\n read_gitfile_gently\n repo_set_gitdir\n setup_git_env\n set_git_dir_1\n set_git_dir\n\nLater, `set_git_dir()` uses its now-dead parameter:\n !is_absolute_path(path)\n\nFix this by using a dedicated `strbuf` to hold `strbuf_realpath()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/init-db.c | 4 ++--\n cache.h | 2 +-\n environment.c | 11 ++++++++++-\n path.c | 2 +-\n setup.c | 18 +++++++++---------\n 5 files changed, 23 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 944ec77fe10..5bf61a7e056 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -356,12 +356,12 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t\tif (!exist_ok && !stat(real_git_dir, &st))\n \t\t\tdie(_(\"%s already exists\"), real_git_dir);\n \n-\t\tset_git_dir(real_path(real_git_dir));\n+\t\tset_git_dir(real_git_dir, 1);\n \t\tgit_dir = get_git_dir();\n \t\tseparate_git_dir(git_dir, original_git_dir);\n \t}\n \telse {\n-\t\tset_git_dir(real_path(git_dir));\n+\t\tset_git_dir(git_dir, 1);\n \t\tgit_dir = get_git_dir();\n \t}\n \tstartup_info->have_repository = 1;\ndiff --git a/cache.h b/cache.h\nindex 37c899b53f7..8cee257d3d7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -543,7 +543,7 @@ const char *get_git_common_dir(void);\n char *get_object_directory(void);\n char *get_index_file(void);\n char *get_graft_file(struct repository *r);\n-void set_git_dir(const char *path);\n+void set_git_dir(const char *path, int make_realpath);\n int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n int get_common_dir(struct strbuf *sb, const char *gitdir);\n const char *get_git_namespace(void);\ndiff --git a/environment.c b/environment.c\nindex e72a02d0d57..c436de31eef 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -345,11 +345,20 @@ static void update_relative_gitdir(const char *name,\n \tfree(path);\n }\n \n-void set_git_dir(const char *path)\n+void set_git_dir(const char *path, int make_realpath)\n {\n+\tstruct strbuf realpath = STRBUF_INIT;\n+\n+\tif (make_realpath) {\n+\t\tstrbuf_realpath(&realpath, path, 1);\n+\t\tpath = realpath.buf;\n+\t}\n+\n \tset_git_dir_1(path);\n \tif (!is_absolute_path(path))\n \t\tchdir_notify_register(NULL, update_relative_gitdir, NULL);\n+\n+\tstrbuf_release(&realpath);\n }\n \n const char *get_log_output_encoding(void)\ndiff --git a/path.c b/path.c\nindex 88cf5930073..c5a8fe4f0c3 100644\n--- a/path.c\n+++ b/path.c\n@@ -850,7 +850,7 @@ const char *enter_repo(const char *path, int strict)\n \t}\n \n \tif (is_git_directory(\".\")) {\n-\t\tset_git_dir(\".\");\n+\t\tset_git_dir(\".\", 0);\n \t\tcheck_repository_format();\n \t\treturn path;\n \t}\ndiff --git a/setup.c b/setup.c\nindex 4ea7a0b081b..fa4317e707a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -725,7 +725,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\t}\n \n \t\t/* #18, #26 */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n@@ -747,7 +747,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t}\n \telse if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {\n \t\t/* #16d */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n@@ -759,14 +759,14 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \n \t/* both get_git_work_tree() and cwd are already normalized */\n \tif (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n \n \toffset = dir_inside_of(cwd->buf, worktree);\n \tif (offset >= 0) {\t/* cwd inside worktree? */\n-\t\tset_git_dir(real_path(gitdirenv));\n+\t\tset_git_dir(gitdirenv, 1);\n \t\tif (chdir(worktree))\n \t\t\tdie_errno(_(\"cannot chdir to '%s'\"), worktree);\n \t\tstrbuf_addch(cwd, '/');\n@@ -775,7 +775,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t}\n \n \t/* cwd outside worktree */\n-\tset_git_dir(gitdirenv);\n+\tset_git_dir(gitdirenv, 0);\n \tfree(gitfile);\n \treturn NULL;\n }\n@@ -804,7 +804,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \n \t/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */\n \tif (is_bare_repository_cfg > 0) {\n-\t\tset_git_dir(offset == cwd->len ? gitdir : real_path(gitdir));\n+\t\tset_git_dir(gitdir, (offset != cwd->len));\n \t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(_(\"cannot come back to cwd\"));\n \t\treturn NULL;\n@@ -813,7 +813,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t/* #0, #1, #5, #8, #9, #12, #13 */\n \tset_git_work_tree(\".\");\n \tif (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))\n-\t\tset_git_dir(gitdir);\n+\t\tset_git_dir(gitdir, 0);\n \tinside_git_dir = 0;\n \tinside_work_tree = 1;\n \tif (offset >= cwd->len)\n@@ -856,10 +856,10 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n \t\t\tdie_errno(_(\"cannot come back to cwd\"));\n \t\troot_len = offset_1st_component(cwd->buf);\n \t\tstrbuf_setlen(cwd, offset > root_len ? offset : root_len);\n-\t\tset_git_dir(cwd->buf);\n+\t\tset_git_dir(cwd->buf, 0);\n \t}\n \telse\n-\t\tset_git_dir(\".\");\n+\t\tset_git_dir(\".\", 0);\n \treturn NULL;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392920","messageId":"2eeefda3d41e6af1bc61249daf14b42050f0d0c3.1583521397.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.git.1583521396.gitgitgadget@gmail.com","subject":"[PATCH 4/4] get_superproject_working_tree(): return strbuf","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-06T19:03:16Z","receivedAt":"2020-03-06T19:03:23Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nTogether with the previous commits, this commit fully fixes the problem\nof using shared buffer for `real_path()` in `get_superproject_working_tree()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/rev-parse.c | 7 ++++---\n submodule.c | 17 ++++++++---------\n submodule.h | 4 ++--\n 3 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 06ca7175ac7..06056434ed1 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -808,9 +808,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-superproject-working-tree\")) {\n-\t\t\t\tconst char *superproject = get_superproject_working_tree();\n-\t\t\t\tif (superproject)\n-\t\t\t\t\tputs(superproject);\n+\t\t\t\tstruct strbuf superproject = STRBUF_INIT;\n+\t\t\t\tif (get_superproject_working_tree(&superproject))\n+\t\t\t\t\tputs(superproject.buf);\n+\t\t\t\tstrbuf_release(&superproject);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\ndiff --git a/submodule.c b/submodule.c\nindex 215c62580fc..46f6c2cbfd0 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2168,14 +2168,13 @@ void absorb_git_dir_into_superproject(const char *path,\n \t}\n }\n \n-const char *get_superproject_working_tree(void)\n+int get_superproject_working_tree(struct strbuf* buf)\n {\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf one_up = STRBUF_INIT;\n \tconst char *cwd = xgetcwd();\n-\tconst char *ret = NULL;\n+\tint ret = 0;\n \tconst char *subpath;\n \tint code;\n \tssize_t len;\n@@ -2186,10 +2185,10 @@ const char *get_superproject_working_tree(void)\n \t\t * We might have a superproject, but it is harder\n \t\t * to determine.\n \t\t */\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tif (!strbuf_realpath(&one_up, \"../\", 0))\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tsubpath = relative_path(cwd, one_up.buf, &sb);\n \tstrbuf_release(&one_up);\n@@ -2233,8 +2232,8 @@ const char *get_superproject_working_tree(void)\n \t\tsuper_wt = xstrdup(cwd);\n \t\tsuper_wt[cwd_len - super_sub_len] = '\\0';\n \n-\t\tstrbuf_realpath(&realpath, super_wt, 1);\n-\t\tret = realpath.buf;\n+\t\tstrbuf_realpath(buf, super_wt, 1);\n+\t\tret = 1;\n \t\tfree(super_wt);\n \t}\n \tstrbuf_release(&sb);\n@@ -2243,10 +2242,10 @@ const char *get_superproject_working_tree(void)\n \n \tif (code == 128)\n \t\t/* '../' is not a git repository */\n-\t\treturn NULL;\n+\t\treturn 0;\n \tif (code == 0 && len == 0)\n \t\t/* There is an unrelated git repository at '../' */\n-\t\treturn NULL;\n+\t\treturn 0;\n \tif (code)\n \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \ndiff --git a/submodule.h b/submodule.h\nindex c81ec1a9b6c..17492e478fc 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -152,8 +152,8 @@ void absorb_git_dir_into_superproject(const char *path,\n /*\n * Return the absolute path of the working tree of the superproject, which this\n * project is a submodule of. If this repository is not a submodule of\n- * another repository, return NULL.\n+ * another repository, return 0.\n */\n-const char *get_superproject_working_tree(void);\n+int get_superproject_working_tree(struct strbuf* buf);\n \n #endif\n-- \ngitgitgadget\n"},{"id":"392921","messageId":"039d3d368662f3a7e208fa7aa47549ca2654574a.1583521396.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.git.1583521396.gitgitgadget@gmail.com","subject":"[PATCH 2/4] real_path: remove unsafe API","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-06T19:03:14Z","receivedAt":"2020-03-06T19:03:25Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nReturning a shared buffer invites very subtle bugs due to reentrancy or\nmulti-threading, as demonstrated by the previous patch.\n\nThere was an unfinished effort to abolish this [1].\n\nLet's finally rid of `real_path()`, using `strbuf_realpath()` instead.\n\nThis patch uses a local `strbuf` for most places where `real_path()` was\npreviously called.\n\nHowever, two places return the value of `real_path()` to the caller. For\nthem, a `static` local `strbuf` was added, effectively pushing the\nproblem one level higher:\n read_gitfile_gently()\n get_superproject_working_tree()\n\n[1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n abspath.c | 8 +-------\n builtin/clone.c | 7 ++++++-\n builtin/commit-graph.c | 6 +++++-\n builtin/rev-parse.c | 5 ++++-\n builtin/worktree.c | 10 +++++++---\n cache.h | 1 -\n editor.c | 11 +++++++++--\n environment.c | 7 ++++++-\n path.c | 2 +-\n setup.c | 17 ++++++++++++++---\n submodule.c | 4 +++-\n t/helper/test-path-utils.c | 6 +++++-\n worktree.c | 5 ++++-\n 13 files changed, 65 insertions(+), 24 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 98579853299..d34026bfeb8 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -206,12 +206,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n * Resolve `path` into an absolute, cleaned-up path. The return value\n * comes from a shared buffer.\n */\n-const char *real_path(const char *path)\n-{\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n-\treturn strbuf_realpath(&realpath, path, 1);\n-}\n-\n const char *real_path_if_valid(const char *path)\n {\n \tstatic struct strbuf realpath = STRBUF_INIT;\n@@ -233,7 +227,7 @@ char *real_pathdup(const char *path, int die_on_error)\n \n /*\n * Use this to get an absolute path from a relative one. If you want\n- * to resolve links, you should use real_path.\n+ * to resolve links, you should use strbuf_realpath.\n */\n const char *absolute_path(const char *path)\n {\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 1ad26f4d8c8..e5c2a229a11 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -420,6 +420,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \tstruct dir_iterator *iter;\n \tint iter_status;\n \tunsigned int flags;\n+\tstruct strbuf realpath = STRBUF_INIT;\n \n \tmkdir_if_missing(dest->buf, 0777);\n \n@@ -454,7 +455,9 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tif (unlink(dest->buf) && errno != ENOENT)\n \t\t\tdie_errno(_(\"failed to unlink '%s'\"), dest->buf);\n \t\tif (!option_no_hardlinks) {\n-\t\t\tif (!link(real_path(src->buf), dest->buf))\n+\t\t\tstrbuf_reset(&realpath);\n+\t\t\tstrbuf_realpath(&realpath, src->buf, 1);\n+\t\t\tif (!link(realpath.buf, dest->buf))\n \t\t\t\tcontinue;\n \t\t\tif (option_local > 0)\n \t\t\t\tdie_errno(_(\"failed to create link '%s'\"), dest->buf);\n@@ -468,6 +471,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tstrbuf_setlen(src, src_len);\n \t\tdie(_(\"failed to iterate over '%s'\"), src->buf);\n \t}\n+\n+\tstrbuf_release(&realpath);\n }\n \n static void clone_local(const char *src_repo, const char *dest_repo)\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 4a70b33fb5f..3d7ec640e01 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -39,14 +39,18 @@ static struct object_directory *find_odb(struct repository *r,\n {\n \tstruct object_directory *odb;\n \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n+\tstruct strbuf odb_path_real = STRBUF_INIT;\n \n \tprepare_alt_odb(r);\n \tfor (odb = r->objects->odb; odb; odb = odb->next) {\n-\t\tif (!strcmp(obj_dir_real, real_path(odb->path)))\n+\t\tstrbuf_reset(&odb_path_real);\n+\t\tstrbuf_realpath(&odb_path_real, odb->path, 1);\n+\t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n \t\t\tbreak;\n \t}\n \n \tfree(obj_dir_real);\n+\tstrbuf_release(&odb_path_real);\n \n \tif (!odb)\n \t\tdie(_(\"could not find object directory matching %s\"), obj_dir);\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 7a00da82035..06ca7175ac7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -857,7 +857,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tif (!gitdir && !prefix)\n \t\t\t\t\t\tgitdir = \".git\";\n \t\t\t\t\tif (gitdir) {\n-\t\t\t\t\t\tputs(real_path(gitdir));\n+\t\t\t\t\t\tstruct strbuf realpath = STRBUF_INIT;\n+\t\t\t\t\t\tstrbuf_realpath(&realpath, gitdir, 1);\n+\t\t\t\t\t\tputs(realpath.buf);\n+\t\t\t\t\t\tstrbuf_release(&realpath);\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n \t\t\t\t}\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 24f22800f38..b13b88bec62 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -258,7 +258,7 @@ static int add_worktree(const char *path, const char *refname,\n \t\t\tconst struct add_opts *opts)\n {\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT;\n \tconst char *name;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct argv_array child_env = ARGV_ARRAY_INIT;\n@@ -330,9 +330,12 @@ static int add_worktree(const char *path, const char *refname,\n \n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n-\twrite_file(sb.buf, \"%s\", real_path(sb_git.buf));\n+\tstrbuf_realpath(&realpath, sb_git.buf, 1);\n+\twrite_file(sb.buf, \"%s\", realpath.buf);\n+\tstrbuf_reset(&realpath);\n+\tstrbuf_realpath(&realpath, get_git_common_dir(), 1);\n \twrite_file(sb_git.buf, \"gitdir: %s/worktrees/%s\",\n-\t\t real_path(get_git_common_dir()), name);\n+\t\t realpath.buf, name);\n \t/*\n \t * This is to keep resolve_ref() happy. We need a valid HEAD\n \t * or is_git_directory() will reject the directory. Any value which\n@@ -418,6 +421,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstrbuf_release(&sb_repo);\n \tstrbuf_release(&sb_git);\n \tstrbuf_release(&sb_name);\n+\tstrbuf_release(&realpath);\n \treturn ret;\n }\n \ndiff --git a/cache.h b/cache.h\nindex 8cee257d3d7..f6937793ec2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path)\n int is_directory(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t int die_on_error);\n-const char *real_path(const char *path);\n const char *real_path_if_valid(const char *path);\n char *real_pathdup(const char *path, int die_on_error);\n const char *absolute_path(const char *path);\ndiff --git a/editor.c b/editor.c\nindex f079abbf110..91989ee8a11 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -54,7 +54,8 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n \n \tif (strcmp(editor, \":\")) {\n-\t\tconst char *args[] = { editor, real_path(path), NULL };\n+\t\tstruct strbuf realpath = STRBUF_INIT;\n+\t\tconst char *args[] = { editor, NULL, NULL };\n \t\tstruct child_process p = CHILD_PROCESS_INIT;\n \t\tint ret, sig;\n \t\tint print_waiting_for_editor = advice_waiting_for_editor && isatty(2);\n@@ -75,16 +76,22 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\t\tfflush(stderr);\n \t\t}\n \n+\t\tstrbuf_realpath(&realpath, path, 1);\n+\t\targs[1] = realpath.buf;\n+\n \t\tp.argv = args;\n \t\tp.env = env;\n \t\tp.use_shell = 1;\n \t\tp.trace2_child_class = \"editor\";\n-\t\tif (start_command(&p) < 0)\n+\t\tif (start_command(&p) < 0) {\n+\t\t\tstrbuf_release(&realpath);\n \t\t\treturn error(\"unable to start editor '%s'\", editor);\n+\t\t}\n \n \t\tsigchain_push(SIGINT, SIG_IGN);\n \t\tsigchain_push(SIGQUIT, SIG_IGN);\n \t\tret = finish_command(&p);\n+\t\tstrbuf_release(&realpath);\n \t\tsig = ret - 128;\n \t\tsigchain_pop(SIGINT);\n \t\tsigchain_pop(SIGQUIT);\ndiff --git a/environment.c b/environment.c\nindex c436de31eef..10c9061c432 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -254,8 +254,11 @@ static int git_work_tree_initialized;\n */\n void set_git_work_tree(const char *new_work_tree)\n {\n+\tstruct strbuf realpath = STRBUF_INIT;\n+\n \tif (git_work_tree_initialized) {\n-\t\tnew_work_tree = real_path(new_work_tree);\n+\t\tstrbuf_realpath(&realpath, new_work_tree, 1);\n+\t\tnew_work_tree = realpath.buf;\n \t\tif (strcmp(new_work_tree, the_repository->worktree))\n \t\t\tdie(\"internal error: work tree has already been set\\n\"\n \t\t\t \"Current worktree: %s\\nNew worktree: %s\",\n@@ -264,6 +267,8 @@ void set_git_work_tree(const char *new_work_tree)\n \t}\n \tgit_work_tree_initialized = 1;\n \trepo_set_worktree(the_repository, new_work_tree);\n+\n+\tstrbuf_release(&realpath);\n }\n \n const char *get_git_work_tree(void)\ndiff --git a/path.c b/path.c\nindex c5a8fe4f0c3..0a42ceb3fb5 100644\n--- a/path.c\n+++ b/path.c\n@@ -723,7 +723,7 @@ static struct passwd *getpw_str(const char *username, size_t len)\n * then it is a newly allocated string. Returns NULL on getpw failure or\n * if path is NULL.\n *\n- * If real_home is true, real_path($HOME) is used in the expansion.\n+ * If real_home is true, strbuf_realpath($HOME) is used in the expansion.\n */\n char *expand_user_path(const char *path, int real_home)\n {\ndiff --git a/setup.c b/setup.c\nindex fa4317e707a..19dded55788 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -32,6 +32,7 @@ static int abspath_part_inside_repo(char *path)\n \tchar *path0;\n \tint off;\n \tconst char *work_tree = get_git_work_tree();\n+\tstruct strbuf realpath = STRBUF_INIT;\n \n \tif (!work_tree)\n \t\treturn -1;\n@@ -60,8 +61,11 @@ static int abspath_part_inside_repo(char *path)\n \t\tpath++;\n \t\tif (*path == '/') {\n \t\t\t*path = '\\0';\n-\t\t\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n+\t\t\tstrbuf_reset(&realpath);\n+\t\t\tstrbuf_realpath(&realpath, path0, 1);\n+\t\t\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t\t\tmemmove(path0, path + 1, len - (path - path0));\n+\t\t\t\tstrbuf_release(&realpath);\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\t*path = '/';\n@@ -69,11 +73,15 @@ static int abspath_part_inside_repo(char *path)\n \t}\n \n \t/* check whole path */\n-\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n+\tstrbuf_reset(&realpath);\n+\tstrbuf_realpath(&realpath, path0, 1);\n+\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t*path0 = '\\0';\n+\t\tstrbuf_release(&realpath);\n \t\treturn 0;\n \t}\n \n+\tstrbuf_release(&realpath);\n \treturn -1;\n }\n \n@@ -619,6 +627,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstruct stat st;\n \tint fd;\n \tssize_t len;\n+\tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n \t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n@@ -669,7 +678,9 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \t\terror_code = READ_GITFILE_ERR_NOT_A_REPO;\n \t\tgoto cleanup_return;\n \t}\n-\tpath = real_path(dir);\n+\n+\tstrbuf_realpath(&realpath, dir, 1);\n+\tpath = realpath.buf;\n \n cleanup_return:\n \tif (return_error_code)\ndiff --git a/submodule.c b/submodule.c\nindex 31f391d7d25..bad7a788c06 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2170,6 +2170,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \n const char *get_superproject_working_tree(void)\n {\n+\tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *one_up = real_path_if_valid(\"../\");\n@@ -2231,7 +2232,8 @@ const char *get_superproject_working_tree(void)\n \t\tsuper_wt = xstrdup(cwd);\n \t\tsuper_wt[cwd_len - super_sub_len] = '\\0';\n \n-\t\tret = real_path(super_wt);\n+\t\tstrbuf_realpath(&realpath, super_wt, 1);\n+\t\tret = realpath.buf;\n \t\tfree(super_wt);\n \t}\n \tstrbuf_release(&sb);\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex 409034cf4ee..40548d31dfe 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -290,11 +290,15 @@ int cmd__path_utils(int argc, const char **argv)\n \t}\n \n \tif (argc >= 2 && !strcmp(argv[1], \"real_path\")) {\n+\t\tstruct strbuf realpath = STRBUF_INIT;\n \t\twhile (argc > 2) {\n-\t\t\tputs(real_path(argv[2]));\n+\t\t\tstrbuf_reset(&realpath);\n+\t\t\tstrbuf_realpath(&realpath, argv[2], 1);\n+\t\t\tputs(realpath.buf);\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n+\t\tstrbuf_release(&realpath);\n \t\treturn 0;\n \t}\n \ndiff --git a/worktree.c b/worktree.c\nindex eba4fd3a038..e7bbf716f6b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -285,6 +285,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n \t\t unsigned flags)\n {\n \tstruct strbuf wt_path = STRBUF_INIT;\n+\tstruct strbuf realpath = STRBUF_INIT;\n \tchar *path = NULL;\n \tint err, ret = -1;\n \n@@ -336,7 +337,8 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n \t\tgoto done;\n \t}\n \n-\tret = fspathcmp(path, real_path(git_common_path(\"worktrees/%s\", wt->id)));\n+\tstrbuf_realpath(&realpath, git_common_path(\"worktrees/%s\", wt->id), 1);\n+\tret = fspathcmp(path, realpath.buf);\n \n \tif (ret)\n \t\tstrbuf_addf_gently(errmsg, _(\"'%s' does not point back to '%s'\"),\n@@ -344,6 +346,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n done:\n \tfree(path);\n \tstrbuf_release(&wt_path);\n+\tstrbuf_release(&realpath);\n \treturn ret;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392922","messageId":"59af49ad9f6b2ffc87e350f9bc00d233f2a9010f.1583521396.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.git.1583521396.gitgitgadget@gmail.com","subject":"[PATCH 3/4] real_path_if_valid(): remove unsafe API","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-06T19:03:15Z","receivedAt":"2020-03-06T19:03:26Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis commit continues the work started with previous commit.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n abspath.c | 10 ----------\n cache.h | 1 -\n setup.c | 2 +-\n sha1-file.c | 13 ++++---------\n submodule.c | 7 ++++---\n worktree.c | 8 ++++++--\n 6 files changed, 15 insertions(+), 26 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex d34026bfeb8..6f15a418bb6 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -202,16 +202,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \treturn retval;\n }\n \n-/*\n- * Resolve `path` into an absolute, cleaned-up path. The return value\n- * comes from a shared buffer.\n- */\n-const char *real_path_if_valid(const char *path)\n-{\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n-\treturn strbuf_realpath(&realpath, path, 0);\n-}\n-\n char *real_pathdup(const char *path, int die_on_error)\n {\n \tstruct strbuf realpath = STRBUF_INIT;\ndiff --git a/cache.h b/cache.h\nindex f6937793ec2..aa3f5ce718a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path)\n int is_directory(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t int die_on_error);\n-const char *real_path_if_valid(const char *path);\n char *real_pathdup(const char *path, int die_on_error);\n const char *absolute_path(const char *path);\n char *absolute_pathdup(const char *path);\ndiff --git a/setup.c b/setup.c\nindex 19dded55788..d319f499d6b 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -888,7 +888,7 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n \n /*\n * A \"string_list_each_func_t\" function that canonicalizes an entry\n- * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n+ * from GIT_CEILING_DIRECTORIES using real_pathdup(), or\n * discards it if unusable. The presence of an empty entry in\n * GIT_CEILING_DIRECTORIES turns off canonicalization for all\n * subsequent entries.\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 616886799e5..f2b24654895 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -676,20 +676,15 @@ void add_to_alternates_memory(const char *reference)\n char *compute_alternate_path(const char *path, struct strbuf *err)\n {\n \tchar *ref_git = NULL;\n-\tconst char *repo, *ref_git_s;\n+\tconst char *repo;\n \tint seen_error = 0;\n \n-\tref_git_s = real_path_if_valid(path);\n-\tif (!ref_git_s) {\n+\tref_git = real_pathdup(path, 0);\n+\tif (!ref_git) {\n \t\tseen_error = 1;\n \t\tstrbuf_addf(err, _(\"path '%s' does not exist\"), path);\n \t\tgoto out;\n-\t} else\n-\t\t/*\n-\t\t * Beware: read_gitfile(), real_path() and mkpath()\n-\t\t * return static buffer\n-\t\t */\n-\t\tref_git = xstrdup(ref_git_s);\n+\t}\n \n \trepo = read_gitfile(ref_git);\n \tif (!repo)\ndiff --git a/submodule.c b/submodule.c\nindex bad7a788c06..215c62580fc 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2173,7 +2173,7 @@ const char *get_superproject_working_tree(void)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *one_up = real_path_if_valid(\"../\");\n+\tstruct strbuf one_up = STRBUF_INIT;\n \tconst char *cwd = xgetcwd();\n \tconst char *ret = NULL;\n \tconst char *subpath;\n@@ -2188,10 +2188,11 @@ const char *get_superproject_working_tree(void)\n \t\t */\n \t\treturn NULL;\n \n-\tif (!one_up)\n+\tif (!strbuf_realpath(&one_up, \"../\", 0))\n \t\treturn NULL;\n \n-\tsubpath = relative_path(cwd, one_up, &sb);\n+\tsubpath = relative_path(cwd, one_up.buf, &sb);\n+\tstrbuf_release(&one_up);\n \n \tprepare_submodule_repo_env(&cp.env_array);\n \targv_array_pop(&cp.env_array);\ndiff --git a/worktree.c b/worktree.c\nindex e7bbf716f6b..2a340fa939b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -226,17 +226,21 @@ struct worktree *find_worktree(struct worktree **list,\n \n struct worktree *find_worktree_by_path(struct worktree **list, const char *p)\n {\n+\tstruct strbuf wt_path = STRBUF_INIT;\n \tchar *path = real_pathdup(p, 0);\n \n \tif (!path)\n \t\treturn NULL;\n \tfor (; *list; list++) {\n-\t\tconst char *wt_path = real_path_if_valid((*list)->path);\n+\t\tstrbuf_reset(&wt_path);\n+\t\tif (!strbuf_realpath(&wt_path, (*list)->path, 0))\n+\t\t\tcontinue;\n \n-\t\tif (wt_path && !fspathcmp(path, wt_path))\n+\t\tif (!fspathcmp(path, wt_path.buf))\n \t\t\tbreak;\n \t}\n \tfree(path);\n+\tstrbuf_release(&wt_path);\n \treturn *list;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392927","messageId":"xmqqa74t2lpr.fsf@gitster-ct.c.googlers.com","threadId":"52949","inReplyTo":"f7afcb4cc83a955b04283475facc02349207557c.1583521396.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 1/4] set_git_dir: fix crash when used with real_path()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-06T21:54:24Z","receivedAt":"2020-03-06T21:54:36Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> From: Alexandr Miloslavskiy \n>\n> `real_path()` returns result from a shared buffer, inviting subtle\n> reentrance bugs. One of these bugs occur when invoked this way:\n> set_git_dir(real_path(git_dir))\n>\n> In this case, `real_path()` has reentrance:\n> real_path\n> read_gitfile_gently\n> repo_set_gitdir\n> setup_git_env\n> set_git_dir_1\n> set_git_dir\n>\n> Later, `set_git_dir()` uses its now-dead parameter:\n> !is_absolute_path(path)\n>\n> Fix this by using a dedicated `strbuf` to hold `strbuf_realpath()`.\n\nWith this detailed explanation, I expected to see a test or two that\ndemonstrates a breakage, but reading a stale value may not\nreproducibly give the same wrong result or crash the program,\nperhaps?\n\n> -void set_git_dir(const char *path)\n> +void set_git_dir(const char *path, int make_realpath)\n> {\n> +\tstruct strbuf realpath = STRBUF_INIT;\n> +\n> +\tif (make_realpath) {\n> +\t\tstrbuf_realpath(&realpath, path, 1);\n> +\t\tpath = realpath.buf;\n> +\t}\n> +\n> \tset_git_dir_1(path);\n> \tif (!is_absolute_path(path))\n> \t\tchdir_notify_register(NULL, update_relative_gitdir, NULL);\n> +\n> +\tstrbuf_release(&realpath);\n> }\n\nMakes sense. I looked at changes to the callers in this patch and\nit all made sense.\n"},{"id":"392928","messageId":"xmqq4kv12kvx.fsf@gitster-ct.c.googlers.com","threadId":"52949","inReplyTo":"039d3d368662f3a7e208fa7aa47549ca2654574a.1583521396.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 2/4] real_path: remove unsafe API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-06T22:12:18Z","receivedAt":"2020-03-06T22:12:25Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> However, two places return the value of `real_path()` to the caller. For\n> them, a `static` local `strbuf` was added, effectively pushing the\n> problem one level higher:\n> read_gitfile_gently()\n> get_superproject_working_tree()\n\nYeah, I noticed that while reading the patch. It is not making it\nany worse, and other parts of the patch made tons of sense (except\none small thing).\n\nIt was especially pleasing to see that care has been taken to avoid\nintroducing strbuf leaks.\n\n\n> diff --git a/builtin/clone.c b/builtin/clone.c\n> index 1ad26f4d8c8..e5c2a229a11 100644\n> --- a/builtin/clone.c\n> +++ b/builtin/clone.c\n> @@ -420,6 +420,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n> \tstruct dir_iterator *iter;\n> \tint iter_status;\n> \tunsigned int flags;\n> +\tstruct strbuf realpath = STRBUF_INIT;\n> \n> \tmkdir_if_missing(dest->buf, 0777);\n> \n> @@ -454,7 +455,9 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n> \t\tif (unlink(dest->buf) && errno != ENOENT)\n> \t\t\tdie_errno(_(\"failed to unlink '%s'\"), dest->buf);\n> \t\tif (!option_no_hardlinks) {\n> -\t\t\tif (!link(real_path(src->buf), dest->buf))\n> +\t\t\tstrbuf_reset(&realpath);\n> +\t\t\tstrbuf_realpath(&realpath, src->buf, 1);\n\nThis is inside a loop, so \"struct strbuf realpath\" here in the\nsecond or subsequent iteration may not be empty; it is true that\nstrbuf_reset() is necessary _somewhere_ in the loop to discard\nthe path that was created for the previous iteration.\n\nIf my reading of the code is correct, however, the first thing that\nis done by strbuf_realpath() is to empty the output buffer by using\nstrbuf_reset() indirectly via get_root_part(). Calling strbuf_reset()\nhere should not hurt, but it is unnecessary, I would think. An even\nworse effect such a redundant strbuf_reset() has is that by repeatedly\nseeing the \"reset then call realpath\" pattern, readers who do not read\nthe implementation of strbuf_realpath() might mistakenly think that\n\n\tstrbuf_addf(&message, \"the path '%s' is really \", path);\n\tstrbuf_realpath(&message, path);\n\nis how realpath() is expected to be used, i.e. keep the current\ncontents in the buffer and append the resolved path to it.\n\n\n> static void clone_local(const char *src_repo, const char *dest_repo)\n> diff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\n> index 4a70b33fb5f..3d7ec640e01 100644\n> --- a/builtin/commit-graph.c\n> +++ b/builtin/commit-graph.c\n> @@ -39,14 +39,18 @@ static struct object_directory *find_odb(struct repository *r,\n> {\n> \tstruct object_directory *odb;\n> \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n> +\tstruct strbuf odb_path_real = STRBUF_INIT;\n> \n> \tprepare_alt_odb(r);\n> \tfor (odb = r->objects->odb; odb; odb = odb->next) {\n> -\t\tif (!strcmp(obj_dir_real, real_path(odb->path)))\n> +\t\tstrbuf_reset(&odb_path_real);\n> +\t\tstrbuf_realpath(&odb_path_real, odb->path, 1);\n\nLikewise.\n\n> @@ -60,8 +61,11 @@ static int abspath_part_inside_repo(char *path)\n> \t\tpath++;\n> \t\tif (*path == '/') {\n> \t\t\t*path = '\\0';\n> -\t\t\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n> +\t\t\tstrbuf_reset(&realpath);\n> +\t\t\tstrbuf_realpath(&realpath, path0, 1);\n\nLikewise.\n\n> @@ -69,11 +73,15 @@ static int abspath_part_inside_repo(char *path)\n> \t}\n> \n> \t/* check whole path */\n> -\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n> +\tstrbuf_reset(&realpath);\n> +\tstrbuf_realpath(&realpath, path0, 1);\n\nLikewise.\n\n> diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\n> index 409034cf4ee..40548d31dfe 100644\n> --- a/t/helper/test-path-utils.c\n> +++ b/t/helper/test-path-utils.c\n> @@ -290,11 +290,15 @@ int cmd__path_utils(int argc, const char **argv)\n> \t}\n> \n> \tif (argc >= 2 && !strcmp(argv[1], \"real_path\")) {\n> +\t\tstruct strbuf realpath = STRBUF_INIT;\n> \t\twhile (argc > 2) {\n> -\t\t\tputs(real_path(argv[2]));\n> +\t\t\tstrbuf_reset(&realpath);\n> +\t\t\tstrbuf_realpath(&realpath, argv[2], 1);\n> +\t\t\tputs(realpath.buf);\n\nLikewise.\n\n\nThanks.\n\n"},{"id":"392929","messageId":"xmqqzhct167f.fsf@gitster-ct.c.googlers.com","threadId":"52949","inReplyTo":"59af49ad9f6b2ffc87e350f9bc00d233f2a9010f.1583521396.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 3/4] real_path_if_valid(): remove unsafe API","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-06T22:14:44Z","receivedAt":"2020-03-06T22:14:57Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> diff --git a/sha1-file.c b/sha1-file.c\n> index 616886799e5..f2b24654895 100644\n> --- a/sha1-file.c\n> +++ b/sha1-file.c\n> @@ -676,20 +676,15 @@ void add_to_alternates_memory(const char *reference)\n> char *compute_alternate_path(const char *path, struct strbuf *err)\n> {\n> \tchar *ref_git = NULL;\n> -\tconst char *repo, *ref_git_s;\n> +\tconst char *repo;\n> \tint seen_error = 0;\n> \n> -\tref_git_s = real_path_if_valid(path);\n> -\tif (!ref_git_s) {\n> +\tref_git = real_pathdup(path, 0);\n> +\tif (!ref_git) {\n> \t\tseen_error = 1;\n> \t\tstrbuf_addf(err, _(\"path '%s' does not exist\"), path);\n> \t\tgoto out;\n> -\t} else\n> -\t\t/*\n> -\t\t * Beware: read_gitfile(), real_path() and mkpath()\n> -\t\t * return static buffer\n> -\t\t */\n> -\t\tref_git = xstrdup(ref_git_s);\n> +\t}\n\nIt is amusing to see that rewriting not to use the unsafe function\nmakes the code a lot easier to follow ;-)\n"},{"id":"392930","messageId":"8cfa5434-4f67-fa1a-7de0-2c4d12653488@syntevo.com","threadId":"52949","inReplyTo":"xmqqa74t2lpr.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 1/4] set_git_dir: fix crash when used with real_path()","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-03-06T22:42:44Z","receivedAt":"2020-03-06T22:42:50Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 06.03.2020 22:54, Junio C Hamano wrote:\n> With this detailed explanation, I expected to see a test or two that\n> demonstrates a breakage, but reading a stale value may not\n> reproducibly give the same wrong result or crash the program,\n> perhaps?\n\nLet's put it this way: one of the tests hits the bug every single time,\nyet still the bug has gone unnoticed for years. So yes, it's not super\nreliable. I think I could make a test that crashes often enough, but\nthe effort will probably not be justified. The problem here is rather\napparent when a finger is pointed to it.\n"},{"id":"392931","messageId":"xmqqv9nh14u0.fsf@gitster-ct.c.googlers.com","threadId":"52949","inReplyTo":"2eeefda3d41e6af1bc61249daf14b42050f0d0c3.1583521397.git.gitgitgadget@gmail.com","subject":"Re: [PATCH 4/4] get_superproject_working_tree(): return strbuf","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-03-06T22:44:23Z","receivedAt":"2020-03-06T22:44:28Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"Alexandr Miloslavskiy via GitGitGadget\" \nwrites:\n\n> \t\t\tif (!strcmp(arg, \"--show-superproject-working-tree\")) {\n> -\t\t\t\tconst char *superproject = get_superproject_working_tree();\n> -\t\t\t\tif (superproject)\n> -\t\t\t\t\tputs(superproject);\n> +\t\t\t\tstruct strbuf superproject = STRBUF_INIT;\n> +\t\t\t\tif (get_superproject_working_tree(&superproject))\n> +\t\t\t\t\tputs(superproject.buf);\n> +\t\t\t\tstrbuf_release(&superproject);\n\nThe new calling convention makes sense here.\n\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\n> diff --git a/submodule.c b/submodule.c\n> index 215c62580fc..46f6c2cbfd0 100644\n> --- a/submodule.c\n> +++ b/submodule.c\n> @@ -2168,14 +2168,13 @@ void absorb_git_dir_into_superproject(const char *path,\n> \t}\n> }\n> \n> -const char *get_superproject_working_tree(void)\n> +int get_superproject_working_tree(struct strbuf* buf)\n\nMicronit. \n\nThe asterisk sticks to the identifier, not type, in our codebase.\nI.e. \"struct strbuf *buf\".\n\n> diff --git a/submodule.h b/submodule.h\n> index c81ec1a9b6c..17492e478fc 100644\n> --- a/submodule.h\n> +++ b/submodule.h\n> @@ -152,8 +152,8 @@ void absorb_git_dir_into_superproject(const char *path,\n> /*\n> * Return the absolute path of the working tree of the superproject, which this\n> * project is a submodule of. If this repository is not a submodule of\n> - * another repository, return NULL.\n> + * another repository, return 0.\n> */\n> -const char *get_superproject_working_tree(void);\n> +int get_superproject_working_tree(struct strbuf* buf);\n\nLikewise.\n\nThe conversion of the function body looked quite sensible.\n\nThanks.\n"},{"id":"392932","messageId":"c1c3c6ec-6361-5711-15e8-01e5ccdb651f@syntevo.com","threadId":"52949","inReplyTo":"xmqq4kv12kvx.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 2/4] real_path: remove unsafe API","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-03-06T22:54:38Z","receivedAt":"2020-03-06T22:54:42Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 06.03.2020 23:12, Junio C Hamano wrote:\n> If my reading of the code is correct, however, the first thing that\n> is done by strbuf_realpath() is to empty the output buffer by using\n> strbuf_reset() indirectly via get_root_part(). Calling strbuf_reset()\n> here should not hurt, but it is unnecessary, I would think. An even\n> worse effect such a redundant strbuf_reset() has is that by repeatedly\n> seeing the \"reset then call realpath\" pattern, readers who do not read\n> the implementation of strbuf_realpath() might mistakenly think that\n> \n> \tstrbuf_addf(&message, \"the path '%s' is really \", path);\n> \tstrbuf_realpath(&message, path);\n> \n> is how realpath() is expected to be used, i.e. keep the current\n> contents in the buffer and append the resolved path to it.\n\nThanks, will change in V2 next week.\n"},{"id":"392933","messageId":"664d6312-0189-8646-3d73-d6869c6a6806@syntevo.com","threadId":"52949","inReplyTo":"xmqqv9nh14u0.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 4/4] get_superproject_working_tree(): return strbuf","fromName":"Alexandr Miloslavskiy","fromEmail":"alexandr.miloslavskiy@syntevo.com","sentAt":"2020-03-06T23:06:06Z","receivedAt":"2020-03-06T23:06:12Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"On 06.03.2020 23:44, Junio C Hamano wrote:\n> Micronit.\n> \n> The asterisk sticks to the identifier, not type, in our codebase.\n> I.e. \"struct strbuf *buf\".\n\nSorry, having difficulties switching between many different styles.\nWill fix in V2 next week.\n\n\nThank you very much for giving such a quick response! It is a great \npleasure when my patches get movement instead of just gathering dust \nlike in some other opensource projects :(\n"},{"id":"392989","messageId":"41950069a169c68e7e6d93f1a7d80166cb3a4689.1583845884.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.v2.git.1583845884.gitgitgadget@gmail.com","subject":"[PATCH v2 4/4] get_superproject_working_tree(): return strbuf","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-10T13:11:24Z","receivedAt":"2020-03-10T13:17:32Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nTogether with the previous commits, this commit fully fixes the problem\nof using shared buffer for `real_path()` in `get_superproject_working_tree()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/rev-parse.c | 7 ++++---\n submodule.c | 17 ++++++++---------\n submodule.h | 4 ++--\n 3 files changed, 14 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 06ca7175ac7..06056434ed1 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -808,9 +808,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-superproject-working-tree\")) {\n-\t\t\t\tconst char *superproject = get_superproject_working_tree();\n-\t\t\t\tif (superproject)\n-\t\t\t\t\tputs(superproject);\n+\t\t\t\tstruct strbuf superproject = STRBUF_INIT;\n+\t\t\t\tif (get_superproject_working_tree(&superproject))\n+\t\t\t\t\tputs(superproject.buf);\n+\t\t\t\tstrbuf_release(&superproject);\n \t\t\t\tcontinue;\n \t\t\t}\n \t\t\tif (!strcmp(arg, \"--show-prefix\")) {\ndiff --git a/submodule.c b/submodule.c\nindex 215c62580fc..c3aadf3fff8 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2168,14 +2168,13 @@ void absorb_git_dir_into_superproject(const char *path,\n \t}\n }\n \n-const char *get_superproject_working_tree(void)\n+int get_superproject_working_tree(struct strbuf *buf)\n {\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tstruct strbuf one_up = STRBUF_INIT;\n \tconst char *cwd = xgetcwd();\n-\tconst char *ret = NULL;\n+\tint ret = 0;\n \tconst char *subpath;\n \tint code;\n \tssize_t len;\n@@ -2186,10 +2185,10 @@ const char *get_superproject_working_tree(void)\n \t\t * We might have a superproject, but it is harder\n \t\t * to determine.\n \t\t */\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tif (!strbuf_realpath(&one_up, \"../\", 0))\n-\t\treturn NULL;\n+\t\treturn 0;\n \n \tsubpath = relative_path(cwd, one_up.buf, &sb);\n \tstrbuf_release(&one_up);\n@@ -2233,8 +2232,8 @@ const char *get_superproject_working_tree(void)\n \t\tsuper_wt = xstrdup(cwd);\n \t\tsuper_wt[cwd_len - super_sub_len] = '\\0';\n \n-\t\tstrbuf_realpath(&realpath, super_wt, 1);\n-\t\tret = realpath.buf;\n+\t\tstrbuf_realpath(buf, super_wt, 1);\n+\t\tret = 1;\n \t\tfree(super_wt);\n \t}\n \tstrbuf_release(&sb);\n@@ -2243,10 +2242,10 @@ const char *get_superproject_working_tree(void)\n \n \tif (code == 128)\n \t\t/* '../' is not a git repository */\n-\t\treturn NULL;\n+\t\treturn 0;\n \tif (code == 0 && len == 0)\n \t\t/* There is an unrelated git repository at '../' */\n-\t\treturn NULL;\n+\t\treturn 0;\n \tif (code)\n \t\tdie(_(\"ls-tree returned unexpected return code %d\"), code);\n \ndiff --git a/submodule.h b/submodule.h\nindex c81ec1a9b6c..4dad649f942 100644\n--- a/submodule.h\n+++ b/submodule.h\n@@ -152,8 +152,8 @@ void absorb_git_dir_into_superproject(const char *path,\n /*\n * Return the absolute path of the working tree of the superproject, which this\n * project is a submodule of. If this repository is not a submodule of\n- * another repository, return NULL.\n+ * another repository, return 0.\n */\n-const char *get_superproject_working_tree(void);\n+int get_superproject_working_tree(struct strbuf *buf);\n \n #endif\n-- \ngitgitgadget\n"},{"id":"392990","messageId":"a49176386710de97bfe92defd92b7861ef7242fb.1583845884.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.v2.git.1583845884.gitgitgadget@gmail.com","subject":"[PATCH v2 3/4] real_path_if_valid(): remove unsafe API","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-10T13:11:23Z","receivedAt":"2020-03-10T13:17:35Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nThis commit continues the work started with previous commit.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n abspath.c | 10 ----------\n cache.h | 1 -\n setup.c | 2 +-\n sha1-file.c | 13 ++++---------\n submodule.c | 7 ++++---\n worktree.c | 7 +++++--\n 6 files changed, 14 insertions(+), 26 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex d34026bfeb8..6f15a418bb6 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -202,16 +202,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \treturn retval;\n }\n \n-/*\n- * Resolve `path` into an absolute, cleaned-up path. The return value\n- * comes from a shared buffer.\n- */\n-const char *real_path_if_valid(const char *path)\n-{\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n-\treturn strbuf_realpath(&realpath, path, 0);\n-}\n-\n char *real_pathdup(const char *path, int die_on_error)\n {\n \tstruct strbuf realpath = STRBUF_INIT;\ndiff --git a/cache.h b/cache.h\nindex f6937793ec2..aa3f5ce718a 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path)\n int is_directory(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t int die_on_error);\n-const char *real_path_if_valid(const char *path);\n char *real_pathdup(const char *path, int die_on_error);\n const char *absolute_path(const char *path);\n char *absolute_pathdup(const char *path);\ndiff --git a/setup.c b/setup.c\nindex 1ae3f203016..9e8fa46bc78 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -886,7 +886,7 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_\n \n /*\n * A \"string_list_each_func_t\" function that canonicalizes an entry\n- * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or\n+ * from GIT_CEILING_DIRECTORIES using real_pathdup(), or\n * discards it if unusable. The presence of an empty entry in\n * GIT_CEILING_DIRECTORIES turns off canonicalization for all\n * subsequent entries.\ndiff --git a/sha1-file.c b/sha1-file.c\nindex 616886799e5..f2b24654895 100644\n--- a/sha1-file.c\n+++ b/sha1-file.c\n@@ -676,20 +676,15 @@ void add_to_alternates_memory(const char *reference)\n char *compute_alternate_path(const char *path, struct strbuf *err)\n {\n \tchar *ref_git = NULL;\n-\tconst char *repo, *ref_git_s;\n+\tconst char *repo;\n \tint seen_error = 0;\n \n-\tref_git_s = real_path_if_valid(path);\n-\tif (!ref_git_s) {\n+\tref_git = real_pathdup(path, 0);\n+\tif (!ref_git) {\n \t\tseen_error = 1;\n \t\tstrbuf_addf(err, _(\"path '%s' does not exist\"), path);\n \t\tgoto out;\n-\t} else\n-\t\t/*\n-\t\t * Beware: read_gitfile(), real_path() and mkpath()\n-\t\t * return static buffer\n-\t\t */\n-\t\tref_git = xstrdup(ref_git_s);\n+\t}\n \n \trepo = read_gitfile(ref_git);\n \tif (!repo)\ndiff --git a/submodule.c b/submodule.c\nindex bad7a788c06..215c62580fc 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2173,7 +2173,7 @@ const char *get_superproject_working_tree(void)\n \tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n-\tconst char *one_up = real_path_if_valid(\"../\");\n+\tstruct strbuf one_up = STRBUF_INIT;\n \tconst char *cwd = xgetcwd();\n \tconst char *ret = NULL;\n \tconst char *subpath;\n@@ -2188,10 +2188,11 @@ const char *get_superproject_working_tree(void)\n \t\t */\n \t\treturn NULL;\n \n-\tif (!one_up)\n+\tif (!strbuf_realpath(&one_up, \"../\", 0))\n \t\treturn NULL;\n \n-\tsubpath = relative_path(cwd, one_up, &sb);\n+\tsubpath = relative_path(cwd, one_up.buf, &sb);\n+\tstrbuf_release(&one_up);\n \n \tprepare_submodule_repo_env(&cp.env_array);\n \targv_array_pop(&cp.env_array);\ndiff --git a/worktree.c b/worktree.c\nindex e7bbf716f6b..543472f0c7b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -226,17 +226,20 @@ struct worktree *find_worktree(struct worktree **list,\n \n struct worktree *find_worktree_by_path(struct worktree **list, const char *p)\n {\n+\tstruct strbuf wt_path = STRBUF_INIT;\n \tchar *path = real_pathdup(p, 0);\n \n \tif (!path)\n \t\treturn NULL;\n \tfor (; *list; list++) {\n-\t\tconst char *wt_path = real_path_if_valid((*list)->path);\n+\t\tif (!strbuf_realpath(&wt_path, (*list)->path, 0))\n+\t\t\tcontinue;\n \n-\t\tif (wt_path && !fspathcmp(path, wt_path))\n+\t\tif (!fspathcmp(path, wt_path.buf))\n \t\t\tbreak;\n \t}\n \tfree(path);\n+\tstrbuf_release(&wt_path);\n \treturn *list;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392991","messageId":"29e7133dcd9321a68019fbaf066c18c11190adef.1583845884.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.v2.git.1583845884.gitgitgadget@gmail.com","subject":"[PATCH v2 2/4] real_path: remove unsafe API","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-10T13:11:22Z","receivedAt":"2020-03-10T13:17:39Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\nReturning a shared buffer invites very subtle bugs due to reentrancy or\nmulti-threading, as demonstrated by the previous patch.\n\nThere was an unfinished effort to abolish this [1].\n\nLet's finally rid of `real_path()`, using `strbuf_realpath()` instead.\n\nThis patch uses a local `strbuf` for most places where `real_path()` was\npreviously called.\n\nHowever, two places return the value of `real_path()` to the caller. For\nthem, a `static` local `strbuf` was added, effectively pushing the\nproblem one level higher:\n read_gitfile_gently()\n get_superproject_working_tree()\n\n[1] https://lore.kernel.org/git/1480964316-99305-1-git-send-email-bmwill@google.com/\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n abspath.c | 8 +-------\n builtin/clone.c | 6 +++++-\n builtin/commit-graph.c | 5 ++++-\n builtin/rev-parse.c | 5 ++++-\n builtin/worktree.c | 9 ++++++---\n cache.h | 1 -\n editor.c | 11 +++++++++--\n environment.c | 7 ++++++-\n path.c | 2 +-\n setup.c | 15 ++++++++++++---\n submodule.c | 4 +++-\n t/helper/test-path-utils.c | 5 ++++-\n worktree.c | 5 ++++-\n 13 files changed, 59 insertions(+), 24 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 98579853299..d34026bfeb8 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -206,12 +206,6 @@ char *strbuf_realpath(struct strbuf *resolved, const char *path,\n * Resolve `path` into an absolute, cleaned-up path. The return value\n * comes from a shared buffer.\n */\n-const char *real_path(const char *path)\n-{\n-\tstatic struct strbuf realpath = STRBUF_INIT;\n-\treturn strbuf_realpath(&realpath, path, 1);\n-}\n-\n const char *real_path_if_valid(const char *path)\n {\n \tstatic struct strbuf realpath = STRBUF_INIT;\n@@ -233,7 +227,7 @@ char *real_pathdup(const char *path, int die_on_error)\n \n /*\n * Use this to get an absolute path from a relative one. If you want\n- * to resolve links, you should use real_path.\n+ * to resolve links, you should use strbuf_realpath.\n */\n const char *absolute_path(const char *path)\n {\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 1ad26f4d8c8..488bdb07417 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -420,6 +420,7 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \tstruct dir_iterator *iter;\n \tint iter_status;\n \tunsigned int flags;\n+\tstruct strbuf realpath = STRBUF_INIT;\n \n \tmkdir_if_missing(dest->buf, 0777);\n \n@@ -454,7 +455,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tif (unlink(dest->buf) && errno != ENOENT)\n \t\t\tdie_errno(_(\"failed to unlink '%s'\"), dest->buf);\n \t\tif (!option_no_hardlinks) {\n-\t\t\tif (!link(real_path(src->buf), dest->buf))\n+\t\t\tstrbuf_realpath(&realpath, src->buf, 1);\n+\t\t\tif (!link(realpath.buf, dest->buf))\n \t\t\t\tcontinue;\n \t\t\tif (option_local > 0)\n \t\t\t\tdie_errno(_(\"failed to create link '%s'\"), dest->buf);\n@@ -468,6 +470,8 @@ static void copy_or_link_directory(struct strbuf *src, struct strbuf *dest,\n \t\tstrbuf_setlen(src, src_len);\n \t\tdie(_(\"failed to iterate over '%s'\"), src->buf);\n \t}\n+\n+\tstrbuf_release(&realpath);\n }\n \n static void clone_local(const char *src_repo, const char *dest_repo)\ndiff --git a/builtin/commit-graph.c b/builtin/commit-graph.c\nindex 4a70b33fb5f..d1ab6625f63 100644\n--- a/builtin/commit-graph.c\n+++ b/builtin/commit-graph.c\n@@ -39,14 +39,17 @@ static struct object_directory *find_odb(struct repository *r,\n {\n \tstruct object_directory *odb;\n \tchar *obj_dir_real = real_pathdup(obj_dir, 1);\n+\tstruct strbuf odb_path_real = STRBUF_INIT;\n \n \tprepare_alt_odb(r);\n \tfor (odb = r->objects->odb; odb; odb = odb->next) {\n-\t\tif (!strcmp(obj_dir_real, real_path(odb->path)))\n+\t\tstrbuf_realpath(&odb_path_real, odb->path, 1);\n+\t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n \t\t\tbreak;\n \t}\n \n \tfree(obj_dir_real);\n+\tstrbuf_release(&odb_path_real);\n \n \tif (!odb)\n \t\tdie(_(\"could not find object directory matching %s\"), obj_dir);\ndiff --git a/builtin/rev-parse.c b/builtin/rev-parse.c\nindex 7a00da82035..06ca7175ac7 100644\n--- a/builtin/rev-parse.c\n+++ b/builtin/rev-parse.c\n@@ -857,7 +857,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\t\tif (!gitdir && !prefix)\n \t\t\t\t\t\tgitdir = \".git\";\n \t\t\t\t\tif (gitdir) {\n-\t\t\t\t\t\tputs(real_path(gitdir));\n+\t\t\t\t\t\tstruct strbuf realpath = STRBUF_INIT;\n+\t\t\t\t\t\tstrbuf_realpath(&realpath, gitdir, 1);\n+\t\t\t\t\t\tputs(realpath.buf);\n+\t\t\t\t\t\tstrbuf_release(&realpath);\n \t\t\t\t\t\tcontinue;\n \t\t\t\t\t}\n \t\t\t\t}\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 24f22800f38..d99db356684 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -258,7 +258,7 @@ static int add_worktree(const char *path, const char *refname,\n \t\t\tconst struct add_opts *opts)\n {\n \tstruct strbuf sb_git = STRBUF_INIT, sb_repo = STRBUF_INIT;\n-\tstruct strbuf sb = STRBUF_INIT;\n+\tstruct strbuf sb = STRBUF_INIT, realpath = STRBUF_INIT;\n \tconst char *name;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct argv_array child_env = ARGV_ARRAY_INIT;\n@@ -330,9 +330,11 @@ static int add_worktree(const char *path, const char *refname,\n \n \tstrbuf_reset(&sb);\n \tstrbuf_addf(&sb, \"%s/gitdir\", sb_repo.buf);\n-\twrite_file(sb.buf, \"%s\", real_path(sb_git.buf));\n+\tstrbuf_realpath(&realpath, sb_git.buf, 1);\n+\twrite_file(sb.buf, \"%s\", realpath.buf);\n+\tstrbuf_realpath(&realpath, get_git_common_dir(), 1);\n \twrite_file(sb_git.buf, \"gitdir: %s/worktrees/%s\",\n-\t\t real_path(get_git_common_dir()), name);\n+\t\t realpath.buf, name);\n \t/*\n \t * This is to keep resolve_ref() happy. We need a valid HEAD\n \t * or is_git_directory() will reject the directory. Any value which\n@@ -418,6 +420,7 @@ static int add_worktree(const char *path, const char *refname,\n \tstrbuf_release(&sb_repo);\n \tstrbuf_release(&sb_git);\n \tstrbuf_release(&sb_name);\n+\tstrbuf_release(&realpath);\n \treturn ret;\n }\n \ndiff --git a/cache.h b/cache.h\nindex 8cee257d3d7..f6937793ec2 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -1314,7 +1314,6 @@ static inline int is_absolute_path(const char *path)\n int is_directory(const char *);\n char *strbuf_realpath(struct strbuf *resolved, const char *path,\n \t\t int die_on_error);\n-const char *real_path(const char *path);\n const char *real_path_if_valid(const char *path);\n char *real_pathdup(const char *path, int die_on_error);\n const char *absolute_path(const char *path);\ndiff --git a/editor.c b/editor.c\nindex f079abbf110..91989ee8a11 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -54,7 +54,8 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n \n \tif (strcmp(editor, \":\")) {\n-\t\tconst char *args[] = { editor, real_path(path), NULL };\n+\t\tstruct strbuf realpath = STRBUF_INIT;\n+\t\tconst char *args[] = { editor, NULL, NULL };\n \t\tstruct child_process p = CHILD_PROCESS_INIT;\n \t\tint ret, sig;\n \t\tint print_waiting_for_editor = advice_waiting_for_editor && isatty(2);\n@@ -75,16 +76,22 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\t\tfflush(stderr);\n \t\t}\n \n+\t\tstrbuf_realpath(&realpath, path, 1);\n+\t\targs[1] = realpath.buf;\n+\n \t\tp.argv = args;\n \t\tp.env = env;\n \t\tp.use_shell = 1;\n \t\tp.trace2_child_class = \"editor\";\n-\t\tif (start_command(&p) < 0)\n+\t\tif (start_command(&p) < 0) {\n+\t\t\tstrbuf_release(&realpath);\n \t\t\treturn error(\"unable to start editor '%s'\", editor);\n+\t\t}\n \n \t\tsigchain_push(SIGINT, SIG_IGN);\n \t\tsigchain_push(SIGQUIT, SIG_IGN);\n \t\tret = finish_command(&p);\n+\t\tstrbuf_release(&realpath);\n \t\tsig = ret - 128;\n \t\tsigchain_pop(SIGINT);\n \t\tsigchain_pop(SIGQUIT);\ndiff --git a/environment.c b/environment.c\nindex c436de31eef..10c9061c432 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -254,8 +254,11 @@ static int git_work_tree_initialized;\n */\n void set_git_work_tree(const char *new_work_tree)\n {\n+\tstruct strbuf realpath = STRBUF_INIT;\n+\n \tif (git_work_tree_initialized) {\n-\t\tnew_work_tree = real_path(new_work_tree);\n+\t\tstrbuf_realpath(&realpath, new_work_tree, 1);\n+\t\tnew_work_tree = realpath.buf;\n \t\tif (strcmp(new_work_tree, the_repository->worktree))\n \t\t\tdie(\"internal error: work tree has already been set\\n\"\n \t\t\t \"Current worktree: %s\\nNew worktree: %s\",\n@@ -264,6 +267,8 @@ void set_git_work_tree(const char *new_work_tree)\n \t}\n \tgit_work_tree_initialized = 1;\n \trepo_set_worktree(the_repository, new_work_tree);\n+\n+\tstrbuf_release(&realpath);\n }\n \n const char *get_git_work_tree(void)\ndiff --git a/path.c b/path.c\nindex c5a8fe4f0c3..0a42ceb3fb5 100644\n--- a/path.c\n+++ b/path.c\n@@ -723,7 +723,7 @@ static struct passwd *getpw_str(const char *username, size_t len)\n * then it is a newly allocated string. Returns NULL on getpw failure or\n * if path is NULL.\n *\n- * If real_home is true, real_path($HOME) is used in the expansion.\n+ * If real_home is true, strbuf_realpath($HOME) is used in the expansion.\n */\n char *expand_user_path(const char *path, int real_home)\n {\ndiff --git a/setup.c b/setup.c\nindex fa4317e707a..1ae3f203016 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -32,6 +32,7 @@ static int abspath_part_inside_repo(char *path)\n \tchar *path0;\n \tint off;\n \tconst char *work_tree = get_git_work_tree();\n+\tstruct strbuf realpath = STRBUF_INIT;\n \n \tif (!work_tree)\n \t\treturn -1;\n@@ -60,8 +61,10 @@ static int abspath_part_inside_repo(char *path)\n \t\tpath++;\n \t\tif (*path == '/') {\n \t\t\t*path = '\\0';\n-\t\t\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n+\t\t\tstrbuf_realpath(&realpath, path0, 1);\n+\t\t\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t\t\tmemmove(path0, path + 1, len - (path - path0));\n+\t\t\t\tstrbuf_release(&realpath);\n \t\t\t\treturn 0;\n \t\t\t}\n \t\t\t*path = '/';\n@@ -69,11 +72,14 @@ static int abspath_part_inside_repo(char *path)\n \t}\n \n \t/* check whole path */\n-\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n+\tstrbuf_realpath(&realpath, path0, 1);\n+\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t*path0 = '\\0';\n+\t\tstrbuf_release(&realpath);\n \t\treturn 0;\n \t}\n \n+\tstrbuf_release(&realpath);\n \treturn -1;\n }\n \n@@ -619,6 +625,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \tstruct stat st;\n \tint fd;\n \tssize_t len;\n+\tstatic struct strbuf realpath = STRBUF_INIT;\n \n \tif (stat(path, &st)) {\n \t\t/* NEEDSWORK: discern between ENOENT vs other errors */\n@@ -669,7 +676,9 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)\n \t\terror_code = READ_GITFILE_ERR_NOT_A_REPO;\n \t\tgoto cleanup_return;\n \t}\n-\tpath = real_path(dir);\n+\n+\tstrbuf_realpath(&realpath, dir, 1);\n+\tpath = realpath.buf;\n \n cleanup_return:\n \tif (return_error_code)\ndiff --git a/submodule.c b/submodule.c\nindex 31f391d7d25..bad7a788c06 100644\n--- a/submodule.c\n+++ b/submodule.c\n@@ -2170,6 +2170,7 @@ void absorb_git_dir_into_superproject(const char *path,\n \n const char *get_superproject_working_tree(void)\n {\n+\tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n \tstruct strbuf sb = STRBUF_INIT;\n \tconst char *one_up = real_path_if_valid(\"../\");\n@@ -2231,7 +2232,8 @@ const char *get_superproject_working_tree(void)\n \t\tsuper_wt = xstrdup(cwd);\n \t\tsuper_wt[cwd_len - super_sub_len] = '\\0';\n \n-\t\tret = real_path(super_wt);\n+\t\tstrbuf_realpath(&realpath, super_wt, 1);\n+\t\tret = realpath.buf;\n \t\tfree(super_wt);\n \t}\n \tstrbuf_release(&sb);\ndiff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c\nindex 409034cf4ee..313a153209c 100644\n--- a/t/helper/test-path-utils.c\n+++ b/t/helper/test-path-utils.c\n@@ -290,11 +290,14 @@ int cmd__path_utils(int argc, const char **argv)\n \t}\n \n \tif (argc >= 2 && !strcmp(argv[1], \"real_path\")) {\n+\t\tstruct strbuf realpath = STRBUF_INIT;\n \t\twhile (argc > 2) {\n-\t\t\tputs(real_path(argv[2]));\n+\t\t\tstrbuf_realpath(&realpath, argv[2], 1);\n+\t\t\tputs(realpath.buf);\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n+\t\tstrbuf_release(&realpath);\n \t\treturn 0;\n \t}\n \ndiff --git a/worktree.c b/worktree.c\nindex eba4fd3a038..e7bbf716f6b 100644\n--- a/worktree.c\n+++ b/worktree.c\n@@ -285,6 +285,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n \t\t unsigned flags)\n {\n \tstruct strbuf wt_path = STRBUF_INIT;\n+\tstruct strbuf realpath = STRBUF_INIT;\n \tchar *path = NULL;\n \tint err, ret = -1;\n \n@@ -336,7 +337,8 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n \t\tgoto done;\n \t}\n \n-\tret = fspathcmp(path, real_path(git_common_path(\"worktrees/%s\", wt->id)));\n+\tstrbuf_realpath(&realpath, git_common_path(\"worktrees/%s\", wt->id), 1);\n+\tret = fspathcmp(path, realpath.buf);\n \n \tif (ret)\n \t\tstrbuf_addf_gently(errmsg, _(\"'%s' does not point back to '%s'\"),\n@@ -344,6 +346,7 @@ int validate_worktree(const struct worktree *wt, struct strbuf *errmsg,\n done:\n \tfree(path);\n \tstrbuf_release(&wt_path);\n+\tstrbuf_release(&realpath);\n \treturn ret;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392992","messageId":"f7afcb4cc83a955b04283475facc02349207557c.1583845884.git.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.v2.git.1583845884.gitgitgadget@gmail.com","subject":"[PATCH v2 1/4] set_git_dir: fix crash when used with real_path()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-10T13:11:21Z","receivedAt":"2020-03-10T13:17:59Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"From: Alexandr Miloslavskiy \n\n`real_path()` returns result from a shared buffer, inviting subtle\nreentrance bugs. One of these bugs occur when invoked this way:\n set_git_dir(real_path(git_dir))\n\nIn this case, `real_path()` has reentrance:\n real_path\n read_gitfile_gently\n repo_set_gitdir\n setup_git_env\n set_git_dir_1\n set_git_dir\n\nLater, `set_git_dir()` uses its now-dead parameter:\n !is_absolute_path(path)\n\nFix this by using a dedicated `strbuf` to hold `strbuf_realpath()`.\n\nSigned-off-by: Alexandr Miloslavskiy \n---\n builtin/init-db.c | 4 ++--\n cache.h | 2 +-\n environment.c | 11 ++++++++++-\n path.c | 2 +-\n setup.c | 18 +++++++++---------\n 5 files changed, 23 insertions(+), 14 deletions(-)\n\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex 944ec77fe10..5bf61a7e056 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -356,12 +356,12 @@ int init_db(const char *git_dir, const char *real_git_dir,\n \t\tif (!exist_ok && !stat(real_git_dir, &st))\n \t\t\tdie(_(\"%s already exists\"), real_git_dir);\n \n-\t\tset_git_dir(real_path(real_git_dir));\n+\t\tset_git_dir(real_git_dir, 1);\n \t\tgit_dir = get_git_dir();\n \t\tseparate_git_dir(git_dir, original_git_dir);\n \t}\n \telse {\n-\t\tset_git_dir(real_path(git_dir));\n+\t\tset_git_dir(git_dir, 1);\n \t\tgit_dir = get_git_dir();\n \t}\n \tstartup_info->have_repository = 1;\ndiff --git a/cache.h b/cache.h\nindex 37c899b53f7..8cee257d3d7 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -543,7 +543,7 @@ const char *get_git_common_dir(void);\n char *get_object_directory(void);\n char *get_index_file(void);\n char *get_graft_file(struct repository *r);\n-void set_git_dir(const char *path);\n+void set_git_dir(const char *path, int make_realpath);\n int get_common_dir_noenv(struct strbuf *sb, const char *gitdir);\n int get_common_dir(struct strbuf *sb, const char *gitdir);\n const char *get_git_namespace(void);\ndiff --git a/environment.c b/environment.c\nindex e72a02d0d57..c436de31eef 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -345,11 +345,20 @@ static void update_relative_gitdir(const char *name,\n \tfree(path);\n }\n \n-void set_git_dir(const char *path)\n+void set_git_dir(const char *path, int make_realpath)\n {\n+\tstruct strbuf realpath = STRBUF_INIT;\n+\n+\tif (make_realpath) {\n+\t\tstrbuf_realpath(&realpath, path, 1);\n+\t\tpath = realpath.buf;\n+\t}\n+\n \tset_git_dir_1(path);\n \tif (!is_absolute_path(path))\n \t\tchdir_notify_register(NULL, update_relative_gitdir, NULL);\n+\n+\tstrbuf_release(&realpath);\n }\n \n const char *get_log_output_encoding(void)\ndiff --git a/path.c b/path.c\nindex 88cf5930073..c5a8fe4f0c3 100644\n--- a/path.c\n+++ b/path.c\n@@ -850,7 +850,7 @@ const char *enter_repo(const char *path, int strict)\n \t}\n \n \tif (is_git_directory(\".\")) {\n-\t\tset_git_dir(\".\");\n+\t\tset_git_dir(\".\", 0);\n \t\tcheck_repository_format();\n \t\treturn path;\n \t}\ndiff --git a/setup.c b/setup.c\nindex 4ea7a0b081b..fa4317e707a 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -725,7 +725,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t\t}\n \n \t\t/* #18, #26 */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n@@ -747,7 +747,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t}\n \telse if (!git_env_bool(GIT_IMPLICIT_WORK_TREE_ENVIRONMENT, 1)) {\n \t\t/* #16d */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n@@ -759,14 +759,14 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \n \t/* both get_git_work_tree() and cwd are already normalized */\n \tif (!strcmp(cwd->buf, worktree)) { /* cwd == worktree */\n-\t\tset_git_dir(gitdirenv);\n+\t\tset_git_dir(gitdirenv, 0);\n \t\tfree(gitfile);\n \t\treturn NULL;\n \t}\n \n \toffset = dir_inside_of(cwd->buf, worktree);\n \tif (offset >= 0) {\t/* cwd inside worktree? */\n-\t\tset_git_dir(real_path(gitdirenv));\n+\t\tset_git_dir(gitdirenv, 1);\n \t\tif (chdir(worktree))\n \t\t\tdie_errno(_(\"cannot chdir to '%s'\"), worktree);\n \t\tstrbuf_addch(cwd, '/');\n@@ -775,7 +775,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \t}\n \n \t/* cwd outside worktree */\n-\tset_git_dir(gitdirenv);\n+\tset_git_dir(gitdirenv, 0);\n \tfree(gitfile);\n \treturn NULL;\n }\n@@ -804,7 +804,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \n \t/* #16.2, #17.2, #20.2, #21.2, #24, #25, #28, #29 (see t1510) */\n \tif (is_bare_repository_cfg > 0) {\n-\t\tset_git_dir(offset == cwd->len ? gitdir : real_path(gitdir));\n+\t\tset_git_dir(gitdir, (offset != cwd->len));\n \t\tif (chdir(cwd->buf))\n \t\t\tdie_errno(_(\"cannot come back to cwd\"));\n \t\treturn NULL;\n@@ -813,7 +813,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t/* #0, #1, #5, #8, #9, #12, #13 */\n \tset_git_work_tree(\".\");\n \tif (strcmp(gitdir, DEFAULT_GIT_DIR_ENVIRONMENT))\n-\t\tset_git_dir(gitdir);\n+\t\tset_git_dir(gitdir, 0);\n \tinside_git_dir = 0;\n \tinside_work_tree = 1;\n \tif (offset >= cwd->len)\n@@ -856,10 +856,10 @@ static const char *setup_bare_git_dir(struct strbuf *cwd, int offset,\n \t\t\tdie_errno(_(\"cannot come back to cwd\"));\n \t\troot_len = offset_1st_component(cwd->buf);\n \t\tstrbuf_setlen(cwd, offset > root_len ? offset : root_len);\n-\t\tset_git_dir(cwd->buf);\n+\t\tset_git_dir(cwd->buf, 0);\n \t}\n \telse\n-\t\tset_git_dir(\".\");\n+\t\tset_git_dir(\".\", 0);\n \treturn NULL;\n }\n \n-- \ngitgitgadget\n\n"},{"id":"392993","messageId":"pull.575.v2.git.1583845884.gitgitgadget@gmail.com","threadId":"52949","inReplyTo":"pull.575.git.1583521396.gitgitgadget@gmail.com","subject":"[PATCH v2 0/4] Fix bugs related to real_path()","fromName":"Alexandr Miloslavskiy via GitGitGadget","fromEmail":"gitgitgadget@gmail.com","sentAt":"2020-03-10T13:11:20Z","receivedAt":"2020-03-10T13:18:01Z","isPatch":true,"sender":{"key":"alexandr.miloslavskiy@syntevo.com","avatar":null},"body":"Changes since V1\n-------------------\n1) Removed `strbuf_realpath()` that weren't needed\n2) Code style in declaration of `get_superproject_working_tree()`\n\nOriginal description\n-------------------\nThe issue with `real_path()` seems to be long-standing, where multiple\npeople solved parts of it over time. I'm adding another part here\nafter I have discovered a crash related to it.\n\nEven with this step, there are still problems remaining:\n* `read_gitfile_gently()` still uses shared buffer.\n* `absolute_path()` was not removed.\n\nThese issues remain because there're too many code references and I'd like\nto avoid submitting a single topic of a scary size.\n\nAlexandr Miloslavskiy (4):\n set_git_dir: fix crash when used with real_path()\n real_path: remove unsafe API\n real_path_if_valid(): remove unsafe API\n get_superproject_working_tree(): return strbuf\n\n abspath.c | 18 +-----------------\n builtin/clone.c | 6 +++++-\n builtin/commit-graph.c | 5 ++++-\n builtin/init-db.c | 4 ++--\n builtin/rev-parse.c | 12 ++++++++----\n builtin/worktree.c | 9 ++++++---\n cache.h | 4 +---\n editor.c | 11 +++++++++--\n environment.c | 18 ++++++++++++++++--\n path.c | 4 ++--\n setup.c | 35 ++++++++++++++++++++++-------------\n sha1-file.c | 13 ++++---------\n submodule.c | 22 ++++++++++++----------\n submodule.h | 4 ++--\n t/helper/test-path-utils.c | 5 ++++-\n worktree.c | 12 +++++++++---\n 16 files changed, 107 insertions(+), 75 deletions(-)\n\n\nbase-commit: 076cbdcd739aeb33c1be87b73aebae5e43d7bcc5\nPublished-As: https://github.com/gitgitgadget/git/releases/tag/pr-575%2FSyntevoAlex%2F%230205(git)_crash_real_path-v2\nFetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-575/SyntevoAlex/#0205(git)_crash_real_path-v2\nPull-Request: https://github.com/gitgitgadget/git/pull/575\n\nRange-diff vs v1:\n\n 1: f7afcb4cc83 = 1: f7afcb4cc83 set_git_dir: fix crash when used with real_path()\n 2: 039d3d36866 ! 2: 29e7133dcd9 real_path: remove unsafe API\n @@ -64,7 +64,6 @@\n \t\t\tdie_errno(_(\"failed to unlink '%s'\"), dest->buf);\n \t\tif (!option_no_hardlinks) {\n -\t\t\tif (!link(real_path(src->buf), dest->buf))\n -+\t\t\tstrbuf_reset(&realpath);\n +\t\t\tstrbuf_realpath(&realpath, src->buf, 1);\n +\t\t\tif (!link(realpath.buf, dest->buf))\n \t\t\t\tcontinue;\n @@ -92,7 +91,6 @@\n \tprepare_alt_odb(r);\n \tfor (odb = r->objects->odb; odb; odb = odb->next) {\n -\t\tif (!strcmp(obj_dir_real, real_path(odb->path)))\n -+\t\tstrbuf_reset(&odb_path_real);\n +\t\tstrbuf_realpath(&odb_path_real, odb->path, 1);\n +\t\tif (!strcmp(obj_dir_real, odb_path_real.buf))\n \t\t\tbreak;\n @@ -139,7 +137,6 @@\n -\twrite_file(sb.buf, \"%s\", real_path(sb_git.buf));\n +\tstrbuf_realpath(&realpath, sb_git.buf, 1);\n +\twrite_file(sb.buf, \"%s\", realpath.buf);\n -+\tstrbuf_reset(&realpath);\n +\tstrbuf_realpath(&realpath, get_git_common_dir(), 1);\n \twrite_file(sb_git.buf, \"gitdir: %s/worktrees/%s\",\n -\t\t real_path(get_git_common_dir()), name);\n @@ -261,7 +258,6 @@\n \t\tif (*path == '/') {\n \t\t\t*path = '\\0';\n -\t\t\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n -+\t\t\tstrbuf_reset(&realpath);\n +\t\t\tstrbuf_realpath(&realpath, path0, 1);\n +\t\t\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t\t\tmemmove(path0, path + 1, len - (path - path0));\n @@ -274,7 +270,6 @@\n \n \t/* check whole path */\n -\tif (fspathcmp(real_path(path0), work_tree) == 0) {\n -+\tstrbuf_reset(&realpath);\n +\tstrbuf_realpath(&realpath, path0, 1);\n +\tif (fspathcmp(realpath.buf, work_tree) == 0) {\n \t\t*path0 = '\\0';\n @@ -338,7 +333,6 @@\n +\t\tstruct strbuf realpath = STRBUF_INIT;\n \t\twhile (argc > 2) {\n -\t\t\tputs(real_path(argv[2]));\n -+\t\t\tstrbuf_reset(&realpath);\n +\t\t\tstrbuf_realpath(&realpath, argv[2], 1);\n +\t\t\tputs(realpath.buf);\n \t\t\targc--;\n 3: 59af49ad9f6 ! 3: a4917638671 real_path_if_valid(): remove unsafe API\n @@ -122,7 +122,6 @@\n \t\treturn NULL;\n \tfor (; *list; list++) {\n -\t\tconst char *wt_path = real_path_if_valid((*list)->path);\n -+\t\tstrbuf_reset(&wt_path);\n +\t\tif (!strbuf_realpath(&wt_path, (*list)->path, 0))\n +\t\t\tcontinue;\n \n 4: 2eeefda3d41 ! 4: 41950069a16 get_superproject_working_tree(): return strbuf\n @@ -33,7 +33,7 @@\n }\n \n -const char *get_superproject_working_tree(void)\n -+int get_superproject_working_tree(struct strbuf* buf)\n ++int get_superproject_working_tree(struct strbuf *buf)\n {\n -\tstatic struct strbuf realpath = STRBUF_INIT;\n \tstruct child_process cp = CHILD_PROCESS_INIT;\n @@ -94,6 +94,6 @@\n + * another repository, return 0.\n */\n -const char *get_superproject_working_tree(void);\n -+int get_superproject_working_tree(struct strbuf* buf);\n ++int get_superproject_working_tree(struct strbuf *buf);\n \n #endif\n\n-- \ngitgitgadget\n"}]}