{"thread":{"id":"26750","subject":"[PATCH 1/3] make_absolute_path: return the input path if it points to our buffer","startedAt":"2011-03-16T16:06:16Z","lastAt":"2011-03-16T20:51:13Z","messageCount":10,"participants":["Carlos Martín Nieto","Brian Gernhardt","Erik Faye-Lund","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":3},"messages":[{"id":"163482","messageId":"1300291579-25852-1-git-send-email-cmn@elego.de","threadId":"26750","inReplyTo":null,"subject":"[PATCH 0/3] Rename make_*_path with clearer names","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:06:16Z","receivedAt":"2011-03-16T16:06:16Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"The first patch in this series is in fact a memory-access, but they\ntogether logically as enhancements to make_*_path. Just say so if\nit'd be better to split them up.\n\nThe calls have been converted 1-to-1 to use the new names, as the real\nresolved path is what is needed most of the time.\n\nCarlos Martín Nieto (3):\n  make_absolute_path: return the input path if it points to our buffer\n  Name make_*_path functions more accurately\n  Use the new {real,absolute}_path function names\n\n abspath.c              |   26 +++++++++++++++++++++++---\n builtin/clone.c        |   12 ++++++------\n builtin/init-db.c      |    8 ++++----\n builtin/receive-pack.c |    2 +-\n cache.h                |    6 +++---\n dir.c                  |    4 ++--\n environment.c          |    4 ++--\n exec_cmd.c             |    2 +-\n lockfile.c             |    4 ++--\n path.c                 |    2 +-\n setup.c                |   14 ++++++--------\n t/t0000-basic.sh       |   10 +++++-----\n test-path-utils.c      |    4 ++--\n 13 files changed, 58 insertions(+), 40 deletions(-)\n\n-- \n1.7.4.1\n"},{"id":"163481","messageId":"1300291579-25852-2-git-send-email-cmn@elego.de","threadId":"26750","inReplyTo":"1300291579-25852-1-git-send-email-cmn@elego.de","subject":"[PATCH 1/3] make_absolute_path: return the input path if it points to our buffer","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:06:17Z","receivedAt":"2011-03-16T16:06:17Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Some codepaths call make_absolute_path with its own return value as\ninput. In such a cases, return the path immediately.\n\nThis fixes a valgrind-discovered error, whereby we tried to copy a\nstring onto itself.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n abspath.c |    4 ++++\n 1 files changed, 4 insertions(+), 0 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex 91ca00f..ff14068 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -24,6 +24,10 @@ const char *make_absolute_path(const char *path)\n \tchar *last_elem = NULL;\n \tstruct stat st;\n \n+\t/* We've already done it */\n+\tif (path == buf || path == next_buf)\n+\t\treturn path;\n+\n \tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n \t\tdie (\"Too long path: %.*s\", 60, path);\n \n-- \n1.7.4.1\n"},{"id":"163484","messageId":"1300291579-25852-3-git-send-email-cmn@elego.de","threadId":"26750","inReplyTo":"1300291579-25852-1-git-send-email-cmn@elego.de","subject":"[PATCH 2/3] Name make_*_path functions more accurately","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:06:18Z","receivedAt":"2011-03-16T16:06:18Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Rename the make_*_path functions so it's clearer what they do, in\nparticlar make clear what the differnce between make_absolute_path and\nmake_nonrelative_path is by renaming them real_path and absolute_path\nrespectively. make_relative_path has an understandable name and is\nrenamed to relative_path to maintain the name convention.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n abspath.c         |   22 +++++++++++++++++++---\n cache.h           |    6 +++---\n path.c            |    2 +-\n t/t0000-basic.sh  |   10 +++++-----\n test-path-utils.c |    4 ++--\n 5 files changed, 30 insertions(+), 14 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex ff14068..47bc73e 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -14,7 +14,14 @@ int is_directory(const char *path)\n /* We allow \"recursive\" symbolic links. Only within reason, though. */\n #define MAXDEPTH 5\n \n-const char *make_absolute_path(const char *path)\n+/*\n+ * Use this to get the real path, i.e. resolve links. If you want an\n+ * absolute path but don't mind links, use absolute_path.\n+ *\n+ * If path is our buffer, then return path, as it's already what the\n+ * user wants.\n+ */\n+const char *real_path(const char *path)\n {\n \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n \tchar cwd[1024] = \"\";\n@@ -104,13 +111,22 @@ static const char *get_pwd_cwd(void)\n \treturn cwd;\n }\n \n-const char *make_nonrelative_path(const char *path)\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+ *\n+ * If the path is already absolute, then return path. As the user is\n+ * never meant to free the return value, we're safe.\n+ */\n+const char *absolute_path(const char *path)\n {\n \tstatic char buf[PATH_MAX + 1];\n \n \tif (is_absolute_path(path)) {\n-\t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n+\t\tif (strlen(path) >= PATH_MAX)\n \t\t\tdie(\"Too long path: %.*s\", 60, path);\n+\t\telse\n+\t\t\treturn path;\n \t} else {\n \t\tsize_t len;\n \t\tconst char *fmt;\ndiff --git a/cache.h b/cache.h\nindex c7b0a28..dbc87be 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -716,9 +716,9 @@ static inline int is_absolute_path(const char *path)\n \treturn path[0] == '/' || has_dos_drive_prefix(path);\n }\n int is_directory(const char *);\n-const char *make_absolute_path(const char *path);\n-const char *make_nonrelative_path(const char *path);\n-const char *make_relative_path(const char *abs, const char *base);\n+const char *real_path(const char *path);\n+const char *absolute_path(const char *path);\n+const char *relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n char *strip_path_suffix(const char *path, const char *suffix);\ndiff --git a/path.c b/path.c\nindex 8951333..4d73cc9 100644\n--- a/path.c\n+++ b/path.c\n@@ -397,7 +397,7 @@ int set_shared_perm(const char *path, int mode)\n \treturn 0;\n }\n \n-const char *make_relative_path(const char *abs, const char *base)\n+const char *relative_path(const char *abs, const char *base)\n {\n \tstatic char buf[PATH_MAX + 1];\n \tint i = 0, j = 0;\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 8deec75..f4e8f43 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -435,7 +435,7 @@ test_expect_success 'update-index D/F conflict' '\n \ttest $numpath0 = 1\n '\n \n-test_expect_success SYMLINKS 'absolute path works as expected' '\n+test_expect_success SYMLINKS 'real path works as expected' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n \tmkdir second &&\n@@ -443,14 +443,14 @@ test_expect_success SYMLINKS 'absolute path works as expected' '\n \tmkdir third &&\n \tdir=\"$(cd .git; pwd -P)\" &&\n \tdir2=third/../second/other/.git &&\n-\ttest \"$dir\" = \"$(test-path-utils make_absolute_path $dir2)\" &&\n+\ttest \"$dir\" = \"$(test-path-utils real_path $dir2)\" &&\n \tfile=\"$dir\"/index &&\n-\ttest \"$file\" = \"$(test-path-utils make_absolute_path $dir2/index)\" &&\n+\ttest \"$file\" = \"$(test-path-utils real_path $dir2/index)\" &&\n \tbasename=blub &&\n-\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils make_absolute_path \"$basename\")\" &&\n+\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils real_path \"$basename\")\" &&\n \tln -s ../first/file .git/syml &&\n \tsym=\"$(cd first; pwd -P)\"/file &&\n-\ttest \"$sym\" = \"$(test-path-utils make_absolute_path \"$dir2/syml\")\"\n+\ttest \"$sym\" = \"$(test-path-utils real_path \"$dir2/syml\")\"\n '\n \n test_expect_success 'very long name in the index handled sanely' '\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex d261398..e767159 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -11,9 +11,9 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n-\tif (argc >= 2 && !strcmp(argv[1], \"make_absolute_path\")) {\n+\tif (argc >= 2 && !strcmp(argv[1], \"real_path\")) {\n \t\twhile (argc > 2) {\n-\t\t\tputs(make_absolute_path(argv[2]));\n+\t\t\tputs(real_path(argv[2]));\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n-- \n1.7.4.1\n"},{"id":"163483","messageId":"1300291579-25852-4-git-send-email-cmn@elego.de","threadId":"26750","inReplyTo":"1300291579-25852-1-git-send-email-cmn@elego.de","subject":"[PATCH 3/3] Use the new {real,absolute}_path function names","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:06:19Z","receivedAt":"2011-03-16T16:06:19Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Use the new names for path functions in the code. Replace uses of\nmake_absolute_path with real_path, make_nonrelative_path with\nabsolute_path and make_relative_path with relative_path.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n builtin/clone.c        |   12 ++++++------\n builtin/init-db.c      |    8 ++++----\n builtin/receive-pack.c |    2 +-\n dir.c                  |    4 ++--\n environment.c          |    4 ++--\n exec_cmd.c             |    2 +-\n lockfile.c             |    4 ++--\n setup.c                |   14 ++++++--------\n 8 files changed, 24 insertions(+), 26 deletions(-)\n\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 60d9a64..780809d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -100,7 +100,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \t\tpath = mkpath(\"%s%s\", repo, suffix[i]);\n \t\tif (is_directory(path)) {\n \t\t\t*is_bundle = 0;\n-\t\t\treturn xstrdup(make_nonrelative_path(path));\n+\t\t\treturn xstrdup(absolute_path(path));\n \t\t}\n \t}\n \n@@ -109,7 +109,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n \t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n \t\t\t*is_bundle = 1;\n-\t\t\treturn xstrdup(make_nonrelative_path(path));\n+\t\t\treturn xstrdup(absolute_path(path));\n \t\t}\n \t}\n \n@@ -203,7 +203,7 @@ static void setup_reference(const char *repo)\n \tstruct transport *transport;\n \tconst struct ref *extra;\n \n-\tref_git = make_absolute_path(option_reference);\n+\tref_git = real_path(option_reference);\n \n \tif (is_directory(mkpath(\"%s/.git/objects\", ref_git)))\n \t\tref_git = mkpath(\"%s/.git\", ref_git);\n@@ -411,9 +411,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tpath = get_repo_path(repo_name, &is_bundle);\n \tif (path)\n-\t\trepo = xstrdup(make_nonrelative_path(repo_name));\n+\t\trepo = xstrdup(absolute_path(repo_name));\n \telse if (!strchr(repo_name, ':'))\n-\t\trepo = xstrdup(make_absolute_path(repo_name));\n+\t\trepo = xstrdup(real_path(repo_name));\n \telse\n \t\trepo = repo_name;\n \tis_local = path && !is_bundle;\n@@ -466,7 +466,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tif (safe_create_leading_directories_const(git_dir) < 0)\n \t\tdie(\"could not create leading directories of '%s'\", git_dir);\n-\tset_git_dir(make_absolute_path(git_dir));\n+\tset_git_dir(real_path(git_dir));\n \n \tif (0 <= option_verbosity)\n \t\tprintf(\"Cloning into %s%s...\\n\",\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex fbeb380..63cf259 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -501,7 +501,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tconst char *git_dir_parent = strrchr(git_dir, '/');\n \t\tif (git_dir_parent) {\n \t\t\tchar *rel = xstrndup(git_dir, git_dir_parent - git_dir);\n-\t\t\tgit_work_tree_cfg = xstrdup(make_absolute_path(rel));\n+\t\t\tgit_work_tree_cfg = xstrdup(real_path(rel));\n \t\t\tfree(rel);\n \t\t}\n \t\tif (!git_work_tree_cfg) {\n@@ -510,7 +510,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie_errno (\"Cannot access current working directory\");\n \t\t}\n \t\tif (work_tree)\n-\t\t\tset_git_work_tree(make_absolute_path(work_tree));\n+\t\t  set_git_work_tree(real_path(work_tree));\n \t\telse\n \t\t\tset_git_work_tree(git_work_tree_cfg);\n \t\tif (access(get_git_work_tree(), X_OK))\n@@ -519,10 +519,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t}\n \telse {\n \t\tif (work_tree)\n-\t\t\tset_git_work_tree(make_absolute_path(work_tree));\n+\t\t\tset_git_work_tree(real_path(work_tree));\n \t}\n \n-\tset_git_dir(make_absolute_path(git_dir));\n+\tset_git_dir(real_path(git_dir));\n \n \treturn init_db(template_dir, flags);\n }\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 760817d..d883585 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -740,7 +740,7 @@ static int add_refs_from_alternate(struct alternate_object_database *e, void *un\n \tconst struct ref *extra;\n \n \te->name[-1] = '\\0';\n-\tother = xstrdup(make_absolute_path(e->base));\n+\tother = xstrdup(real_path(e->base));\n \te->name[-1] = '/';\n \tlen = strlen(other);\n \ndiff --git a/dir.c b/dir.c\nindex 570b651..5a9372a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1023,8 +1023,8 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)\n \tif (!getcwd(buffer, size))\n \t\tdie_errno(\"can't find the current directory\");\n \n-\tif (!is_absolute_path(dir))\n-\t\tdir = make_absolute_path(dir);\n+\t/* getcwd resolves links and gives us the real path */\n+\tdir = real_path(dir);\n \n \twhile (*dir && *dir == *cwd) {\n \t\tdir++;\ndiff --git a/environment.c b/environment.c\nindex c3efbb9..cc670b1 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -139,7 +139,7 @@ static int git_work_tree_initialized;\n void set_git_work_tree(const char *new_work_tree)\n {\n \tif (git_work_tree_initialized) {\n-\t\tnew_work_tree = make_absolute_path(new_work_tree);\n+\t\tnew_work_tree = real_path(new_work_tree);\n \t\tif (strcmp(new_work_tree, work_tree))\n \t\t\tdie(\"internal error: work tree has already been set\\n\"\n \t\t\t    \"Current worktree: %s\\nNew worktree: %s\",\n@@ -147,7 +147,7 @@ void set_git_work_tree(const char *new_work_tree)\n \t\treturn;\n \t}\n \tgit_work_tree_initialized = 1;\n-\twork_tree = xstrdup(make_absolute_path(new_work_tree));\n+\twork_tree = xstrdup(real_path(new_work_tree));\n }\n \n const char *get_git_work_tree(void)\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 38545e8..171e841 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -89,7 +89,7 @@ static void add_path(struct strbuf *out, const char *path)\n \t\tif (is_absolute_path(path))\n \t\t\tstrbuf_addstr(out, path);\n \t\telse\n-\t\t\tstrbuf_addstr(out, make_nonrelative_path(path));\n+\t\t\tstrbuf_addstr(out, absolute_path(path));\n \n \t\tstrbuf_addch(out, PATH_SEP);\n \t}\ndiff --git a/lockfile.c b/lockfile.c\nindex b0d74cd..c6fb77b 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -164,10 +164,10 @@ static char *unable_to_lock_message(const char *path, int err)\n \t\t    \"If no other git process is currently running, this probably means a\\n\"\n \t\t    \"git process crashed in this repository earlier. Make sure no other git\\n\"\n \t\t    \"process is running and remove the file manually to continue.\",\n-\t\t\t    make_nonrelative_path(path), strerror(err));\n+\t\t\t    absolute_path(path), strerror(err));\n \t} else\n \t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s\",\n-\t\t\t    make_nonrelative_path(path), strerror(err));\n+\t\t\t    absolute_path(path), strerror(err));\n \treturn strbuf_detach(&buf, NULL);\n }\n \ndiff --git a/setup.c b/setup.c\nindex dadc666..8cb1ad3 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -216,9 +216,7 @@ void setup_work_tree(void)\n \tif (initialized)\n \t\treturn;\n \twork_tree = get_git_work_tree();\n-\tgit_dir = get_git_dir();\n-\tif (!is_absolute_path(git_dir))\n-\t\tgit_dir = make_absolute_path(git_dir);\n+\tgit_dir = real_path(get_git_dir());\n \tif (!work_tree || chdir(work_tree))\n \t\tdie(\"This operation must be run in a work tree\");\n \n@@ -229,7 +227,7 @@ void setup_work_tree(void)\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT))\n \t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, \".\", 1);\n \n-\tset_git_dir(make_relative_path(git_dir, work_tree));\n+\tset_git_dir(relative_path(git_dir, work_tree));\n \tinitialized = 1;\n }\n \n@@ -309,7 +307,7 @@ const char *read_gitfile_gently(const char *path)\n \n \tif (!is_git_directory(dir))\n \t\tdie(\"Not a git repository: %s\", dir);\n-\tpath = make_absolute_path(dir);\n+\tpath = real_path(dir);\n \n \tfree(buf);\n \treturn path;\n@@ -389,7 +387,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \n \tif (!prefixcmp(cwd, worktree) &&\n \t    cwd[strlen(worktree)] == '/') { /* cwd inside worktree */\n-\t\tset_git_dir(make_absolute_path(gitdirenv));\n+\t\tset_git_dir(real_path(gitdirenv));\n \t\tif (chdir(worktree))\n \t\t\tdie_errno(\"Could not chdir to '%s'\", worktree);\n \t\tcwd[len++] = '/';\n@@ -414,7 +412,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t/* --work-tree is set without --git-dir; use discovered one */\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n \t\tif (offset != len && !is_absolute_path(gitdir))\n-\t\t\tgitdir = xstrdup(make_absolute_path(gitdir));\n+\t\t\tgitdir = xstrdup(real_path(gitdir));\n \t\tif (chdir(cwd))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n \t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n@@ -422,7 +420,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 == len ? gitdir : make_absolute_path(gitdir));\n+\t\tset_git_dir(offset == len ? gitdir : real_path(gitdir));\n \t\tif (chdir(cwd))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n \t\treturn NULL;\n-- \n1.7.4.1\n"},{"id":"163489","messageId":"AANLkTikvb0-XJKwNmaJGeJiZQzYC=_k9_MChyOgvkE1o@mail.gmail.com","threadId":"26750","inReplyTo":"1300291579-25852-4-git-send-email-cmn@elego.de","subject":"Re: [PATCH 3/3] Use the new {real,absolute}_path function names","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@gmail.com","sentAt":"2011-03-16T16:24:49Z","receivedAt":"2011-03-16T16:24:49Z","isPatch":true,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Wed, Mar 16, 2011 at 5:06 PM, Carlos Martín Nieto <cmn@elego.de> wrote:\n> Use the new names for path functions in the code. Replace uses of\n> make_absolute_path with real_path, make_nonrelative_path with\n> absolute_path and make_relative_path with relative_path.\n\nShouldn't these changes be squashed into the previous two commits so\nit'll be possible to bisect across it without getting a broken build?\n"},{"id":"163487","messageId":"D7B7C57A-B4DB-4CDC-B079-77537D8E8EFD@silverinsanity.com","threadId":"26750","inReplyTo":"1300291579-25852-3-git-send-email-cmn@elego.de","subject":"Re: [PATCH 2/3] Name make_*_path functions more accurately","fromName":"Brian Gernhardt","fromEmail":"benji@silverinsanity.com","sentAt":"2011-03-16T16:29:52Z","receivedAt":"2011-03-16T16:29:52Z","isPatch":true,"sender":{"key":"benji@silverinsanity.com","avatar":"https://gravatar.com/avatar/e06c101dbc25c68114d859b4a9ec7cf8a2c52fd2b0270ef0eac0e2e63ff22311?d=mp&s=160"},"body":"\nOn Mar 16, 2011, at 12:06 PM, Carlos Martín Nieto wrote:\n\n> Rename the make_*_path functions so it's clearer what they do, in\n> particlar make clear what the differnce between make_absolute_path and\n> make_nonrelative_path is by renaming them real_path and absolute_path\n> respectively. make_relative_path has an understandable name and is\n> renamed to relative_path to maintain the name convention.\n> \n> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n\nI didn't try it, but it looks like 2/3 horribly breaks the code and 3/3 fixes it.  I personally (and I think others) prefer patches that are each useful on their own.  Especially since a code-breaking patch like this makes bisecting harder.\n\nI would suggest doing one of the following:\n\n1) Squashing 2/3 and 3/3 so all the renaming occurs at once.\n2) Adding wrappers from the old name to the new in 2/3 and removing them in 3/3.\n\nThat said, I'm not sure the renaming is useful although the documentation comments definitely are.\n\n~~ Brian"},{"id":"163490","messageId":"1300293451.7214.47.camel@bee.lab.cmartin.tk","threadId":"26750","inReplyTo":"AANLkTikvb0-XJKwNmaJGeJiZQzYC=_k9_MChyOgvkE1o@mail.gmail.com","subject":"Re: [PATCH 3/3] Use the new {real,absolute}_path function names","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:37:31Z","receivedAt":"2011-03-16T16:37:31Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On mié, 2011-03-16 at 17:24 +0100, Erik Faye-Lund wrote:\n> On Wed, Mar 16, 2011 at 5:06 PM, Carlos Martín Nieto <cmn@elego.de> wrote:\n> > Use the new names for path functions in the code. Replace uses of\n> > make_absolute_path with real_path, make_nonrelative_path with\n> > absolute_path and make_relative_path with relative_path.\n> \n> Shouldn't these changes be squashed into the previous two commits so\n> it'll be possible to bisect across it without getting a broken build?\n\n Brian pointed this out in the other subthread. I'll resend.\n"},{"id":"163491","messageId":"1300293751.7214.52.camel@bee.lab.cmartin.tk","threadId":"26750","inReplyTo":"D7B7C57A-B4DB-4CDC-B079-77537D8E8EFD@silverinsanity.com","subject":"Re: [PATCH 2/3] Name make_*_path functions more accurately","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:42:25Z","receivedAt":"2011-03-16T16:42:25Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"On mié, 2011-03-16 at 12:29 -0400, Brian Gernhardt wrote:\n> On Mar 16, 2011, at 12:06 PM, Carlos Martín Nieto wrote:\n> \n> > Rename the make_*_path functions so it's clearer what they do, in\n> > particlar make clear what the differnce between make_absolute_path and\n> > make_nonrelative_path is by renaming them real_path and absolute_path\n> > respectively. make_relative_path has an understandable name and is\n> > renamed to relative_path to maintain the name convention.\n> > \n> > Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n> \n> I didn't try it, but it looks like 2/3 horribly breaks the code and\n> 3/3 fixes it.  I personally (and I think others) prefer patches that\n> are each useful on their own.  Especially since a code-breaking patch\n> like this makes bisecting harder.\n\n True enough.\n\n> \n> I would suggest doing one of the following:\n> \n> 1) Squashing 2/3 and 3/3 so all the renaming occurs at once.\n> 2) Adding wrappers from the old name to the new in 2/3 and removing\n> them in 3/3.\n\n I'll squash.\n\n> \n> That said, I'm not sure the renaming is useful although the\n> documentation comments definitely are.\n\n Do you think the difference between make_nonrelative_path and\nmake_absolute_path is clear without looking at the code? For me at\nleast, a relative path is the opposite of an absolute one, and a\nnon-relative path is the opposite of a relative one. To make a\ndifference between absolute and non-relative is then bound to lead to\nerrors.\n\n   cmn\n"},{"id":"163493","messageId":"1300294349-26946-1-git-send-email-cmn@elego.de","threadId":"26750","inReplyTo":"1300291579-25852-1-git-send-email-cmn@elego.de","subject":"[PATCH 2-3/3] Name make_*_path functions more accurately","fromName":"Carlos Martín Nieto","fromEmail":"cmn@elego.de","sentAt":"2011-03-16T16:52:29Z","receivedAt":"2011-03-16T16:52:29Z","isPatch":true,"sender":{"key":"cmn@elego.de","avatar":"https://avatars.githubusercontent.com/u/335443?v=4"},"body":"Rename the make_*_path functions so it's clearer what they do, in\nparticlar make clear what the differnce between make_absolute_path and\nmake_nonrelative_path is by renaming them real_path and absolute_path\nrespectively. make_relative_path has an understandable name and is\nrenamed to relative_path to maintain the name convention.\n\nThe function calls have been replaced 1-to-1 in their usage.\n\nSigned-off-by: Carlos Martín Nieto <cmn@elego.de>\n---\n\nSorry, I sent the other one without in-reply-to information\n\nThis supercedes the 2/3 and 3/3 patches.\n\n abspath.c              |   22 +++++++++++++++++++---\n builtin/clone.c        |   12 ++++++------\n builtin/init-db.c      |    8 ++++----\n builtin/receive-pack.c |    2 +-\n cache.h                |    6 +++---\n dir.c                  |    4 ++--\n environment.c          |    4 ++--\n exec_cmd.c             |    2 +-\n lockfile.c             |    4 ++--\n path.c                 |    2 +-\n setup.c                |   14 ++++++--------\n t/t0000-basic.sh       |   10 +++++-----\n test-path-utils.c      |    4 ++--\n 13 files changed, 54 insertions(+), 40 deletions(-)\n\ndiff --git a/abspath.c b/abspath.c\nindex ff14068..47bc73e 100644\n--- a/abspath.c\n+++ b/abspath.c\n@@ -14,7 +14,14 @@ int is_directory(const char *path)\n /* We allow \"recursive\" symbolic links. Only within reason, though. */\n #define MAXDEPTH 5\n \n-const char *make_absolute_path(const char *path)\n+/*\n+ * Use this to get the real path, i.e. resolve links. If you want an\n+ * absolute path but don't mind links, use absolute_path.\n+ *\n+ * If path is our buffer, then return path, as it's already what the\n+ * user wants.\n+ */\n+const char *real_path(const char *path)\n {\n \tstatic char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1];\n \tchar cwd[1024] = \"\";\n@@ -104,13 +111,22 @@ static const char *get_pwd_cwd(void)\n \treturn cwd;\n }\n \n-const char *make_nonrelative_path(const char *path)\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+ *\n+ * If the path is already absolute, then return path. As the user is\n+ * never meant to free the return value, we're safe.\n+ */\n+const char *absolute_path(const char *path)\n {\n \tstatic char buf[PATH_MAX + 1];\n \n \tif (is_absolute_path(path)) {\n-\t\tif (strlcpy(buf, path, PATH_MAX) >= PATH_MAX)\n+\t\tif (strlen(path) >= PATH_MAX)\n \t\t\tdie(\"Too long path: %.*s\", 60, path);\n+\t\telse\n+\t\t\treturn path;\n \t} else {\n \t\tsize_t len;\n \t\tconst char *fmt;\ndiff --git a/builtin/clone.c b/builtin/clone.c\nindex 60d9a64..780809d 100644\n--- a/builtin/clone.c\n+++ b/builtin/clone.c\n@@ -100,7 +100,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \t\tpath = mkpath(\"%s%s\", repo, suffix[i]);\n \t\tif (is_directory(path)) {\n \t\t\t*is_bundle = 0;\n-\t\t\treturn xstrdup(make_nonrelative_path(path));\n+\t\t\treturn xstrdup(absolute_path(path));\n \t\t}\n \t}\n \n@@ -109,7 +109,7 @@ static char *get_repo_path(const char *repo, int *is_bundle)\n \t\tpath = mkpath(\"%s%s\", repo, bundle_suffix[i]);\n \t\tif (!stat(path, &st) && S_ISREG(st.st_mode)) {\n \t\t\t*is_bundle = 1;\n-\t\t\treturn xstrdup(make_nonrelative_path(path));\n+\t\t\treturn xstrdup(absolute_path(path));\n \t\t}\n \t}\n \n@@ -203,7 +203,7 @@ static void setup_reference(const char *repo)\n \tstruct transport *transport;\n \tconst struct ref *extra;\n \n-\tref_git = make_absolute_path(option_reference);\n+\tref_git = real_path(option_reference);\n \n \tif (is_directory(mkpath(\"%s/.git/objects\", ref_git)))\n \t\tref_git = mkpath(\"%s/.git\", ref_git);\n@@ -411,9 +411,9 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tpath = get_repo_path(repo_name, &is_bundle);\n \tif (path)\n-\t\trepo = xstrdup(make_nonrelative_path(repo_name));\n+\t\trepo = xstrdup(absolute_path(repo_name));\n \telse if (!strchr(repo_name, ':'))\n-\t\trepo = xstrdup(make_absolute_path(repo_name));\n+\t\trepo = xstrdup(real_path(repo_name));\n \telse\n \t\trepo = repo_name;\n \tis_local = path && !is_bundle;\n@@ -466,7 +466,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix)\n \n \tif (safe_create_leading_directories_const(git_dir) < 0)\n \t\tdie(\"could not create leading directories of '%s'\", git_dir);\n-\tset_git_dir(make_absolute_path(git_dir));\n+\tset_git_dir(real_path(git_dir));\n \n \tif (0 <= option_verbosity)\n \t\tprintf(\"Cloning into %s%s...\\n\",\ndiff --git a/builtin/init-db.c b/builtin/init-db.c\nindex fbeb380..63cf259 100644\n--- a/builtin/init-db.c\n+++ b/builtin/init-db.c\n@@ -501,7 +501,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\tconst char *git_dir_parent = strrchr(git_dir, '/');\n \t\tif (git_dir_parent) {\n \t\t\tchar *rel = xstrndup(git_dir, git_dir_parent - git_dir);\n-\t\t\tgit_work_tree_cfg = xstrdup(make_absolute_path(rel));\n+\t\t\tgit_work_tree_cfg = xstrdup(real_path(rel));\n \t\t\tfree(rel);\n \t\t}\n \t\tif (!git_work_tree_cfg) {\n@@ -510,7 +510,7 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t\t\t\tdie_errno (\"Cannot access current working directory\");\n \t\t}\n \t\tif (work_tree)\n-\t\t\tset_git_work_tree(make_absolute_path(work_tree));\n+\t\t  set_git_work_tree(real_path(work_tree));\n \t\telse\n \t\t\tset_git_work_tree(git_work_tree_cfg);\n \t\tif (access(get_git_work_tree(), X_OK))\n@@ -519,10 +519,10 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)\n \t}\n \telse {\n \t\tif (work_tree)\n-\t\t\tset_git_work_tree(make_absolute_path(work_tree));\n+\t\t\tset_git_work_tree(real_path(work_tree));\n \t}\n \n-\tset_git_dir(make_absolute_path(git_dir));\n+\tset_git_dir(real_path(git_dir));\n \n \treturn init_db(template_dir, flags);\n }\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 760817d..d883585 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -740,7 +740,7 @@ static int add_refs_from_alternate(struct alternate_object_database *e, void *un\n \tconst struct ref *extra;\n \n \te->name[-1] = '\\0';\n-\tother = xstrdup(make_absolute_path(e->base));\n+\tother = xstrdup(real_path(e->base));\n \te->name[-1] = '/';\n \tlen = strlen(other);\n \ndiff --git a/cache.h b/cache.h\nindex c7b0a28..dbc87be 100644\n--- a/cache.h\n+++ b/cache.h\n@@ -716,9 +716,9 @@ static inline int is_absolute_path(const char *path)\n \treturn path[0] == '/' || has_dos_drive_prefix(path);\n }\n int is_directory(const char *);\n-const char *make_absolute_path(const char *path);\n-const char *make_nonrelative_path(const char *path);\n-const char *make_relative_path(const char *abs, const char *base);\n+const char *real_path(const char *path);\n+const char *absolute_path(const char *path);\n+const char *relative_path(const char *abs, const char *base);\n int normalize_path_copy(char *dst, const char *src);\n int longest_ancestor_length(const char *path, const char *prefix_list);\n char *strip_path_suffix(const char *path, const char *suffix);\ndiff --git a/dir.c b/dir.c\nindex 570b651..5a9372a 100644\n--- a/dir.c\n+++ b/dir.c\n@@ -1023,8 +1023,8 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)\n \tif (!getcwd(buffer, size))\n \t\tdie_errno(\"can't find the current directory\");\n \n-\tif (!is_absolute_path(dir))\n-\t\tdir = make_absolute_path(dir);\n+\t/* getcwd resolves links and gives us the real path */\n+\tdir = real_path(dir);\n \n \twhile (*dir && *dir == *cwd) {\n \t\tdir++;\ndiff --git a/environment.c b/environment.c\nindex c3efbb9..cc670b1 100644\n--- a/environment.c\n+++ b/environment.c\n@@ -139,7 +139,7 @@ static int git_work_tree_initialized;\n void set_git_work_tree(const char *new_work_tree)\n {\n \tif (git_work_tree_initialized) {\n-\t\tnew_work_tree = make_absolute_path(new_work_tree);\n+\t\tnew_work_tree = real_path(new_work_tree);\n \t\tif (strcmp(new_work_tree, work_tree))\n \t\t\tdie(\"internal error: work tree has already been set\\n\"\n \t\t\t    \"Current worktree: %s\\nNew worktree: %s\",\n@@ -147,7 +147,7 @@ void set_git_work_tree(const char *new_work_tree)\n \t\treturn;\n \t}\n \tgit_work_tree_initialized = 1;\n-\twork_tree = xstrdup(make_absolute_path(new_work_tree));\n+\twork_tree = xstrdup(real_path(new_work_tree));\n }\n \n const char *get_git_work_tree(void)\ndiff --git a/exec_cmd.c b/exec_cmd.c\nindex 38545e8..171e841 100644\n--- a/exec_cmd.c\n+++ b/exec_cmd.c\n@@ -89,7 +89,7 @@ static void add_path(struct strbuf *out, const char *path)\n \t\tif (is_absolute_path(path))\n \t\t\tstrbuf_addstr(out, path);\n \t\telse\n-\t\t\tstrbuf_addstr(out, make_nonrelative_path(path));\n+\t\t\tstrbuf_addstr(out, absolute_path(path));\n \n \t\tstrbuf_addch(out, PATH_SEP);\n \t}\ndiff --git a/lockfile.c b/lockfile.c\nindex b0d74cd..c6fb77b 100644\n--- a/lockfile.c\n+++ b/lockfile.c\n@@ -164,10 +164,10 @@ static char *unable_to_lock_message(const char *path, int err)\n \t\t    \"If no other git process is currently running, this probably means a\\n\"\n \t\t    \"git process crashed in this repository earlier. Make sure no other git\\n\"\n \t\t    \"process is running and remove the file manually to continue.\",\n-\t\t\t    make_nonrelative_path(path), strerror(err));\n+\t\t\t    absolute_path(path), strerror(err));\n \t} else\n \t\tstrbuf_addf(&buf, \"Unable to create '%s.lock': %s\",\n-\t\t\t    make_nonrelative_path(path), strerror(err));\n+\t\t\t    absolute_path(path), strerror(err));\n \treturn strbuf_detach(&buf, NULL);\n }\n \ndiff --git a/path.c b/path.c\nindex 8951333..4d73cc9 100644\n--- a/path.c\n+++ b/path.c\n@@ -397,7 +397,7 @@ int set_shared_perm(const char *path, int mode)\n \treturn 0;\n }\n \n-const char *make_relative_path(const char *abs, const char *base)\n+const char *relative_path(const char *abs, const char *base)\n {\n \tstatic char buf[PATH_MAX + 1];\n \tint i = 0, j = 0;\ndiff --git a/setup.c b/setup.c\nindex dadc666..8cb1ad3 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -216,9 +216,7 @@ void setup_work_tree(void)\n \tif (initialized)\n \t\treturn;\n \twork_tree = get_git_work_tree();\n-\tgit_dir = get_git_dir();\n-\tif (!is_absolute_path(git_dir))\n-\t\tgit_dir = make_absolute_path(git_dir);\n+\tgit_dir = real_path(get_git_dir());\n \tif (!work_tree || chdir(work_tree))\n \t\tdie(\"This operation must be run in a work tree\");\n \n@@ -229,7 +227,7 @@ void setup_work_tree(void)\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT))\n \t\tsetenv(GIT_WORK_TREE_ENVIRONMENT, \".\", 1);\n \n-\tset_git_dir(make_relative_path(git_dir, work_tree));\n+\tset_git_dir(relative_path(git_dir, work_tree));\n \tinitialized = 1;\n }\n \n@@ -309,7 +307,7 @@ const char *read_gitfile_gently(const char *path)\n \n \tif (!is_git_directory(dir))\n \t\tdie(\"Not a git repository: %s\", dir);\n-\tpath = make_absolute_path(dir);\n+\tpath = real_path(dir);\n \n \tfree(buf);\n \treturn path;\n@@ -389,7 +387,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,\n \n \tif (!prefixcmp(cwd, worktree) &&\n \t    cwd[strlen(worktree)] == '/') { /* cwd inside worktree */\n-\t\tset_git_dir(make_absolute_path(gitdirenv));\n+\t\tset_git_dir(real_path(gitdirenv));\n \t\tif (chdir(worktree))\n \t\t\tdie_errno(\"Could not chdir to '%s'\", worktree);\n \t\tcwd[len++] = '/';\n@@ -414,7 +412,7 @@ static const char *setup_discovered_git_dir(const char *gitdir,\n \t/* --work-tree is set without --git-dir; use discovered one */\n \tif (getenv(GIT_WORK_TREE_ENVIRONMENT) || git_work_tree_cfg) {\n \t\tif (offset != len && !is_absolute_path(gitdir))\n-\t\t\tgitdir = xstrdup(make_absolute_path(gitdir));\n+\t\t\tgitdir = xstrdup(real_path(gitdir));\n \t\tif (chdir(cwd))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n \t\treturn setup_explicit_git_dir(gitdir, cwd, len, nongit_ok);\n@@ -422,7 +420,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 == len ? gitdir : make_absolute_path(gitdir));\n+\t\tset_git_dir(offset == len ? gitdir : real_path(gitdir));\n \t\tif (chdir(cwd))\n \t\t\tdie_errno(\"Could not come back to cwd\");\n \t\treturn NULL;\ndiff --git a/t/t0000-basic.sh b/t/t0000-basic.sh\nindex 8deec75..f4e8f43 100755\n--- a/t/t0000-basic.sh\n+++ b/t/t0000-basic.sh\n@@ -435,7 +435,7 @@ test_expect_success 'update-index D/F conflict' '\n \ttest $numpath0 = 1\n '\n \n-test_expect_success SYMLINKS 'absolute path works as expected' '\n+test_expect_success SYMLINKS 'real path works as expected' '\n \tmkdir first &&\n \tln -s ../.git first/.git &&\n \tmkdir second &&\n@@ -443,14 +443,14 @@ test_expect_success SYMLINKS 'absolute path works as expected' '\n \tmkdir third &&\n \tdir=\"$(cd .git; pwd -P)\" &&\n \tdir2=third/../second/other/.git &&\n-\ttest \"$dir\" = \"$(test-path-utils make_absolute_path $dir2)\" &&\n+\ttest \"$dir\" = \"$(test-path-utils real_path $dir2)\" &&\n \tfile=\"$dir\"/index &&\n-\ttest \"$file\" = \"$(test-path-utils make_absolute_path $dir2/index)\" &&\n+\ttest \"$file\" = \"$(test-path-utils real_path $dir2/index)\" &&\n \tbasename=blub &&\n-\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils make_absolute_path \"$basename\")\" &&\n+\ttest \"$dir/$basename\" = \"$(cd .git && test-path-utils real_path \"$basename\")\" &&\n \tln -s ../first/file .git/syml &&\n \tsym=\"$(cd first; pwd -P)\"/file &&\n-\ttest \"$sym\" = \"$(test-path-utils make_absolute_path \"$dir2/syml\")\"\n+\ttest \"$sym\" = \"$(test-path-utils real_path \"$dir2/syml\")\"\n '\n \n test_expect_success 'very long name in the index handled sanely' '\ndiff --git a/test-path-utils.c b/test-path-utils.c\nindex d261398..e767159 100644\n--- a/test-path-utils.c\n+++ b/test-path-utils.c\n@@ -11,9 +11,9 @@ int main(int argc, char **argv)\n \t\treturn 0;\n \t}\n \n-\tif (argc >= 2 && !strcmp(argv[1], \"make_absolute_path\")) {\n+\tif (argc >= 2 && !strcmp(argv[1], \"real_path\")) {\n \t\twhile (argc > 2) {\n-\t\t\tputs(make_absolute_path(argv[2]));\n+\t\t\tputs(real_path(argv[2]));\n \t\t\targc--;\n \t\t\targv++;\n \t\t}\n-- \n1.7.4.1\n"},{"id":"163512","messageId":"7vaagu7pry.fsf@alter.siamese.dyndns.org","threadId":"26750","inReplyTo":"1300291579-25852-2-git-send-email-cmn@elego.de","subject":"Re: [PATCH 1/3] make_absolute_path: return the input path if it points to our buffer","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-03-16T20:51:13Z","receivedAt":"2011-03-16T20:51:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Martín Nieto <cmn@elego.de> writes:\n\n> Some codepaths call make_absolute_path with its own return value as\n> input. In such a cases, return the path immediately.\n>\n> This fixes a valgrind-discovered error, whereby we tried to copy a\n> string onto itself.\n>\n> Signed-off-by: Carlos Martín Nieto <cmn@elego.de>\n> ---\n>  abspath.c |    4 ++++\n>  1 files changed, 4 insertions(+), 0 deletions(-)\n>\n> diff --git a/abspath.c b/abspath.c\n> index 91ca00f..ff14068 100644\n> --- a/abspath.c\n> +++ b/abspath.c\n> @@ -24,6 +24,10 @@ const char *make_absolute_path(const char *path)\n>  \tchar *last_elem = NULL;\n>  \tstruct stat st;\n>  \n> +\t/* We've already done it */\n> +\tif (path == buf || path == next_buf)\n> +\t\treturn path;\n> +\n\nI like this, as it is very obvious what we are checking here.  Thanks.\n"}]}