From 038e55fec2153a8195236d57dced4bbb001ccf3c Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:20 +0100 Subject: [PATCH 1/8] Introduce new static function real_path_internal() It accepts a new parameter, die_on_error. If die_on_error is false, it simply cleans up after itself and returns NULL rather than dying. Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- abspath.c | 93 ++++++++++++++++++++++++++++++++++++++++++------------- 1 file changed, 72 insertions(+), 21 deletions(-) diff --git a/abspath.c b/abspath.c index 05f2d79348..a7ab8e908f 100644 --- a/abspath.c +++ b/abspath.c @@ -15,15 +15,26 @@ int is_directory(const char *path) #define MAXDEPTH 5 /* - * Use this to get the real path, i.e. resolve links. If you want an - * absolute path but don't mind links, use absolute_path. + * Return the real path (i.e., absolute path, with symlinks resolved + * and extra slashes removed) equivalent to the specified path. (If + * you want an absolute path but don't mind links, use + * absolute_path().) The return value is a pointer to a static + * buffer. + * + * The input and all intermediate paths must be shorter than MAX_PATH. + * The directory part of path (i.e., everything up to the last + * dir_sep) must denote a valid, existing directory, but the last + * component need not exist. If die_on_error is set, then die with an + * informative error message if there is a problem. Otherwise, return + * NULL on errors (without generating any output). * * If path is our buffer, then return path, as it's already what the * user wants. */ -const char *real_path(const char *path) +static const char *real_path_internal(const char *path, int die_on_error) { static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1]; + char *retval = NULL; char cwd[1024] = ""; int buf_index = 1; @@ -35,11 +46,19 @@ const char *real_path(const char *path) if (path == buf || path == next_buf) return path; - if (!*path) - die("The empty string is not a valid path"); + if (!*path) { + if (die_on_error) + die("The empty string is not a valid path"); + else + goto error_out; + } - if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) - die ("Too long path: %.*s", 60, path); + if (strlcpy(buf, path, PATH_MAX) >= PATH_MAX) { + if (die_on_error) + die("Too long path: %.*s", 60, path); + else + goto error_out; + } while (depth--) { if (!is_directory(buf)) { @@ -54,20 +73,36 @@ const char *real_path(const char *path) } if (*buf) { - if (!*cwd && !getcwd(cwd, sizeof(cwd))) - die_errno ("Could not get current working directory"); + if (!*cwd && !getcwd(cwd, sizeof(cwd))) { + if (die_on_error) + die_errno("Could not get current working directory"); + else + goto error_out; + } - if (chdir(buf)) - die_errno ("Could not switch to '%s'", buf); + if (chdir(buf)) { + if (die_on_error) + die_errno("Could not switch to '%s'", buf); + else + goto error_out; + } + } + if (!getcwd(buf, PATH_MAX)) { + if (die_on_error) + die_errno("Could not get current working directory"); + else + goto error_out; } - if (!getcwd(buf, PATH_MAX)) - die_errno ("Could not get current working directory"); if (last_elem) { size_t len = strlen(buf); - if (len + strlen(last_elem) + 2 > PATH_MAX) - die ("Too long path name: '%s/%s'", - buf, last_elem); + if (len + strlen(last_elem) + 2 > PATH_MAX) { + if (die_on_error) + die("Too long path name: '%s/%s'", + buf, last_elem); + else + goto error_out; + } if (len && !is_dir_sep(buf[len-1])) buf[len++] = '/'; strcpy(buf + len, last_elem); @@ -77,10 +112,18 @@ const char *real_path(const char *path) if (!lstat(buf, &st) && S_ISLNK(st.st_mode)) { ssize_t len = readlink(buf, next_buf, PATH_MAX); - if (len < 0) - die_errno ("Invalid symlink '%s'", buf); - if (PATH_MAX <= len) - die("symbolic link too long: %s", buf); + if (len < 0) { + if (die_on_error) + die_errno("Invalid symlink '%s'", buf); + else + goto error_out; + } + if (PATH_MAX <= len) { + if (die_on_error) + die("symbolic link too long: %s", buf); + else + goto error_out; + } next_buf[len] = '\0'; buf = next_buf; buf_index = 1 - buf_index; @@ -89,10 +132,18 @@ const char *real_path(const char *path) break; } + retval = buf; +error_out: + free(last_elem); if (*cwd && chdir(cwd)) die_errno ("Could not change back to '%s'", cwd); - return buf; + return retval; +} + +const char *real_path(const char *path) +{ + return real_path_internal(path, 1); } static const char *get_pwd_cwd(void) From d6052abca39fc84fed4f3248be042cfb6bf635d5 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:21 +0100 Subject: [PATCH 2/8] real_path_internal(): add comment explaining use of cwd Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- abspath.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/abspath.c b/abspath.c index a7ab8e908f..f8a526f391 100644 --- a/abspath.c +++ b/abspath.c @@ -35,7 +35,14 @@ static const char *real_path_internal(const char *path, int die_on_error) { static char bufs[2][PATH_MAX + 1], *buf = bufs[0], *next_buf = bufs[1]; char *retval = NULL; + + /* + * If we have to temporarily chdir(), store the original CWD + * here so that we can chdir() back to it at the end of the + * function: + */ char cwd[1024] = ""; + int buf_index = 1; int depth = MAXDEPTH; From e3e46cdbd45c2e7383df9de1787e23489dc66dbc Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:22 +0100 Subject: [PATCH 3/8] Introduce new function real_path_if_valid() The function is like real_path(), except that it returns NULL on error instead of dying. Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- abspath.c | 5 +++++ cache.h | 1 + 2 files changed, 6 insertions(+) diff --git a/abspath.c b/abspath.c index f8a526f391..40cdc46219 100644 --- a/abspath.c +++ b/abspath.c @@ -153,6 +153,11 @@ const char *real_path(const char *path) return real_path_internal(path, 1); } +const char *real_path_if_valid(const char *path) +{ + return real_path_internal(path, 0); +} + static const char *get_pwd_cwd(void) { static char cwd[PATH_MAX + 1]; diff --git a/cache.h b/cache.h index a58df84bd3..b0d75bcb0e 100644 --- a/cache.h +++ b/cache.h @@ -714,6 +714,7 @@ static inline int is_absolute_path(const char *path) } int is_directory(const char *); const char *real_path(const char *path); +const char *real_path_if_valid(const char *path); const char *absolute_path(const char *path); const char *relative_path(const char *abs, const char *base); int normalize_path_copy(char *dst, const char *src); From a5ccdbe416081d5749f286c193b70400e48ad03f Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:23 +0100 Subject: [PATCH 4/8] longest_ancestor_length(): use string_list_split() Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- path.c | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/path.c b/path.c index cbbdf7d6ba..f455e8e63d 100644 --- a/path.c +++ b/path.c @@ -12,6 +12,7 @@ */ #include "cache.h" #include "strbuf.h" +#include "string-list.h" static char bad_path[] = "/bad-path/"; @@ -582,20 +583,22 @@ int normalize_path_copy(char *dst, const char *src) */ int longest_ancestor_length(const char *path, const char *prefix_list) { + struct string_list prefixes = STRING_LIST_INIT_DUP; char buf[PATH_MAX+1]; - const char *ceil, *colon; - int len, max_len = -1; + int i, max_len = -1; if (prefix_list == NULL || !strcmp(path, "/")) return -1; - for (colon = ceil = prefix_list; *colon; ceil = colon+1) { - for (colon = ceil; *colon && *colon != PATH_SEP; colon++); - len = colon - ceil; + string_list_split(&prefixes, prefix_list, PATH_SEP, -1); + + for (i = 0; i < prefixes.nr; i++) { + const char *ceil = prefixes.items[i].string; + int len = strlen(ceil); + if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil)) continue; - strlcpy(buf, ceil, len+1); - if (normalize_path_copy(buf, buf) < 0) + if (normalize_path_copy(buf, ceil) < 0) continue; len = strlen(buf); if (len > 0 && buf[len-1] == '/') @@ -608,6 +611,7 @@ int longest_ancestor_length(const char *path, const char *prefix_list) } } + string_list_clear(&prefixes, 0); return max_len; } From 31171d9e454d71144685866cfd6476b8ac69d314 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:24 +0100 Subject: [PATCH 5/8] longest_ancestor_length(): take a string_list argument for prefixes Change longest_ancestor_length() to take the prefixes argument as a string_list rather than as a colon-separated string. This will make it easier for the caller to alter the entries before calling longest_ancestor_length(). Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- cache.h | 2 +- path.c | 22 +++++++++------------- setup.c | 11 +++++++++-- test-path-utils.c | 8 +++++++- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/cache.h b/cache.h index b0d75bcb0e..810338552d 100644 --- a/cache.h +++ b/cache.h @@ -718,7 +718,7 @@ const char *real_path_if_valid(const char *path); const char *absolute_path(const char *path); const char *relative_path(const char *abs, const char *base); int normalize_path_copy(char *dst, const char *src); -int longest_ancestor_length(const char *path, const char *prefix_list); +int longest_ancestor_length(const char *path, struct string_list *prefixes); char *strip_path_suffix(const char *path, const char *suffix); int daemon_avoid_alias(const char *path); int offset_1st_component(const char *path); diff --git a/path.c b/path.c index f455e8e63d..b80d2e6e4c 100644 --- a/path.c +++ b/path.c @@ -570,30 +570,27 @@ int normalize_path_copy(char *dst, const char *src) /* * path = Canonical absolute path - * prefix_list = Colon-separated list of absolute paths + * prefixes = string_list containing absolute paths * - * Determines, for each path in prefix_list, whether the "prefix" really + * Determines, for each path in prefixes, whether the "prefix" really * is an ancestor directory of path. Returns the length of the longest * ancestor directory, excluding any trailing slashes, or -1 if no prefix - * is an ancestor. (Note that this means 0 is returned if prefix_list is - * "/".) "/foo" is not considered an ancestor of "/foobar". Directories + * is an ancestor. (Note that this means 0 is returned if prefixes is + * ["/"].) "/foo" is not considered an ancestor of "/foobar". Directories * are not considered to be their own ancestors. path must be in a * canonical form: empty components, or "." or ".." components are not - * allowed. prefix_list may be null, which is like "". + * allowed. Empty strings in prefixes are ignored. */ -int longest_ancestor_length(const char *path, const char *prefix_list) +int longest_ancestor_length(const char *path, struct string_list *prefixes) { - struct string_list prefixes = STRING_LIST_INIT_DUP; char buf[PATH_MAX+1]; int i, max_len = -1; - if (prefix_list == NULL || !strcmp(path, "/")) + if (!strcmp(path, "/")) return -1; - string_list_split(&prefixes, prefix_list, PATH_SEP, -1); - - for (i = 0; i < prefixes.nr; i++) { - const char *ceil = prefixes.items[i].string; + for (i = 0; i < prefixes->nr; i++) { + const char *ceil = prefixes->items[i].string; int len = strlen(ceil); if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil)) @@ -611,7 +608,6 @@ int longest_ancestor_length(const char *path, const char *prefix_list) } } - string_list_clear(&prefixes, 0); return max_len; } diff --git a/setup.c b/setup.c index 3a1b2fd455..b4cd356456 100644 --- a/setup.c +++ b/setup.c @@ -1,5 +1,6 @@ #include "cache.h" #include "dir.h" +#include "string-list.h" static int inside_git_dir = -1; static int inside_work_tree = -1; @@ -627,10 +628,11 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_ static const char *setup_git_directory_gently_1(int *nongit_ok) { const char *env_ceiling_dirs = getenv(CEILING_DIRECTORIES_ENVIRONMENT); + struct string_list ceiling_dirs = STRING_LIST_INIT_DUP; static char cwd[PATH_MAX+1]; const char *gitdirenv, *ret; char *gitfile; - int len, offset, offset_parent, ceil_offset; + int len, offset, offset_parent, ceil_offset = -1; dev_t current_device = 0; int one_filesystem = 1; @@ -655,7 +657,12 @@ static const char *setup_git_directory_gently_1(int *nongit_ok) if (gitdirenv) return setup_explicit_git_dir(gitdirenv, cwd, len, nongit_ok); - ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs); + if (env_ceiling_dirs) { + string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1); + ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs); + string_list_clear(&ceiling_dirs, 0); + } + if (ceil_offset < 0 && has_dos_drive_prefix(cwd)) ceil_offset = 1; diff --git a/test-path-utils.c b/test-path-utils.c index 3bc20e91da..acb05600df 100644 --- a/test-path-utils.c +++ b/test-path-utils.c @@ -1,4 +1,5 @@ #include "cache.h" +#include "string-list.h" int main(int argc, char **argv) { @@ -30,7 +31,12 @@ int main(int argc, char **argv) } if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) { - int len = longest_ancestor_length(argv[2], argv[3]); + int len; + struct string_list ceiling_dirs = STRING_LIST_INIT_DUP; + + string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1); + len = longest_ancestor_length(argv[2], &ceiling_dirs); + string_list_clear(&ceiling_dirs, 0); printf("%d\n", len); return 0; } From 9e2326c7e1efb1ae42b17e3fa38c16711a8d0bd8 Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:25 +0100 Subject: [PATCH 6/8] longest_ancestor_length(): require prefix list entries to be normalized Move the responsibility for normalizing prefixes from longest_ancestor_length() to its callers. Use slightly different normalizations at the two callers: In setup_git_directory_gently_1(), use the old normalization, which ignores paths that are not usable. In the next commit we will change this caller to also resolve symlinks in the paths from GIT_CEILING_DIRECTORIES as part of the normalization. In "test-path-utils longest_ancestor_length", use the old normalization, but die() if any paths are unusable. Also change t0060 to only pass normalized paths to the test program (no empty entries or non-absolute paths, strip trailing slashes from the paths, and remove tests that thereby become redundant). The point of this change is to reduce the scope of the ancestor_length tests in t0060 from testing normalization+longest_prefix to testing only mostly longest_prefix. This is necessary because when setup_git_directory_gently_1() starts resolving symlinks as part of its normalization, it will not be reasonable to do the same in the test suite, because that would make the test results depend on the contents of the root directory of the filesystem on which the test is run. HOWEVER: under Windows, bash mangles arguments that look like absolute POSIX paths into DOS paths. So we have to retain the level of normalization done by normalize_path_copy() to convert the bash-mangled DOS paths (which contain backslashes) into paths that use forward slashes. Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- path.c | 26 +++++++++++-------------- setup.c | 23 ++++++++++++++++++++++ t/t0060-path-utils.sh | 41 +++++++++++++-------------------------- test-path-utils.c | 45 ++++++++++++++++++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 44 deletions(-) diff --git a/path.c b/path.c index b80d2e6e4c..d3d3f8b8ad 100644 --- a/path.c +++ b/path.c @@ -570,20 +570,20 @@ int normalize_path_copy(char *dst, const char *src) /* * path = Canonical absolute path - * prefixes = string_list containing absolute paths + * prefixes = string_list containing normalized, absolute paths without + * trailing slashes (except for the root directory, which is denoted by "/"). * - * Determines, for each path in prefixes, whether the "prefix" really + * Determines, for each path in prefixes, whether the "prefix" * is an ancestor directory of path. Returns the length of the longest * ancestor directory, excluding any trailing slashes, or -1 if no prefix * is an ancestor. (Note that this means 0 is returned if prefixes is * ["/"].) "/foo" is not considered an ancestor of "/foobar". Directories * are not considered to be their own ancestors. path must be in a * canonical form: empty components, or "." or ".." components are not - * allowed. Empty strings in prefixes are ignored. + * allowed. */ int longest_ancestor_length(const char *path, struct string_list *prefixes) { - char buf[PATH_MAX+1]; int i, max_len = -1; if (!strcmp(path, "/")) @@ -593,19 +593,15 @@ int longest_ancestor_length(const char *path, struct string_list *prefixes) const char *ceil = prefixes->items[i].string; int len = strlen(ceil); - if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil)) - continue; - if (normalize_path_copy(buf, ceil) < 0) - continue; - len = strlen(buf); - if (len > 0 && buf[len-1] == '/') - buf[--len] = '\0'; + if (len == 1 && ceil[0] == '/') + len = 0; /* root matches anything, with length 0 */ + else if (!strncmp(path, ceil, len) && path[len] == '/') + ; /* match of length len */ + else + continue; /* no match */ - if (!strncmp(path, buf, len) && - path[len] == '/' && - len > max_len) { + if (len > max_len) max_len = len; - } } return max_len; diff --git a/setup.c b/setup.c index b4cd356456..df97ad3976 100644 --- a/setup.c +++ b/setup.c @@ -621,6 +621,28 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_ return buf.st_dev; } +/* + * A "string_list_each_func_t" function that normalizes an entry from + * GIT_CEILING_DIRECTORIES or discards it if unusable. + */ +static int normalize_ceiling_entry(struct string_list_item *item, void *unused) +{ + const char *ceil = item->string; + int len = strlen(ceil); + char buf[PATH_MAX+1]; + + if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil)) + return 0; + if (normalize_path_copy(buf, ceil) < 0) + return 0; + len = strlen(buf); + if (len > 1 && buf[len-1] == '/') + buf[--len] = '\0'; + free(item->string); + item->string = xstrdup(buf); + return 1; +} + /* * We cannot decide in this function whether we are in the work tree or * not, since the config can only be read _after_ this function was called. @@ -659,6 +681,7 @@ static const char *setup_git_directory_gently_1(int *nongit_ok) if (env_ceiling_dirs) { string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1); + filter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL); ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs); string_list_clear(&ceiling_dirs, 0); } diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh index 4ef2345982..09a42a428e 100755 --- a/t/t0060-path-utils.sh +++ b/t/t0060-path-utils.sh @@ -93,47 +93,32 @@ norm_path /d1/s1//../s2/../../d2 /d2 POSIX norm_path /d1/.../d2 /d1/.../d2 POSIX norm_path /d1/..././../d2 /d1/d2 POSIX -ancestor / "" -1 ancestor / / -1 -ancestor /foo "" -1 -ancestor /foo : -1 -ancestor /foo ::. -1 -ancestor /foo ::..:: -1 ancestor /foo / 0 ancestor /foo /fo -1 ancestor /foo /foo -1 -ancestor /foo /foo/ -1 ancestor /foo /bar -1 -ancestor /foo /bar/ -1 ancestor /foo /foo/bar -1 -ancestor /foo /foo:/bar/ -1 -ancestor /foo /foo/:/bar/ -1 -ancestor /foo /foo::/bar/ -1 -ancestor /foo /:/foo:/bar/ 0 -ancestor /foo /foo:/:/bar/ 0 -ancestor /foo /:/bar/:/foo 0 -ancestor /foo/bar "" -1 +ancestor /foo /foo:/bar -1 +ancestor /foo /:/foo:/bar 0 +ancestor /foo /foo:/:/bar 0 +ancestor /foo /:/bar:/foo 0 ancestor /foo/bar / 0 ancestor /foo/bar /fo -1 -ancestor /foo/bar foo -1 ancestor /foo/bar /foo 4 -ancestor /foo/bar /foo/ 4 ancestor /foo/bar /foo/ba -1 ancestor /foo/bar /:/fo 0 ancestor /foo/bar /foo:/foo/ba 4 ancestor /foo/bar /bar -1 -ancestor /foo/bar /bar/ -1 -ancestor /foo/bar /fo: -1 -ancestor /foo/bar :/fo -1 -ancestor /foo/bar /foo:/bar/ 4 -ancestor /foo/bar /:/foo:/bar/ 4 -ancestor /foo/bar /foo:/:/bar/ 4 -ancestor /foo/bar /:/bar/:/fo 0 -ancestor /foo/bar /:/bar/ 0 -ancestor /foo/bar .:/foo/. 4 -ancestor /foo/bar .:/foo/.:.: 4 -ancestor /foo/bar /foo/./:.:/bar 4 -ancestor /foo/bar .:/bar -1 +ancestor /foo/bar /fo -1 +ancestor /foo/bar /foo:/bar 4 +ancestor /foo/bar /:/foo:/bar 4 +ancestor /foo/bar /foo:/:/bar 4 +ancestor /foo/bar /:/bar:/fo 0 +ancestor /foo/bar /:/bar 0 +ancestor /foo/bar /foo 4 +ancestor /foo/bar /foo:/bar 4 +ancestor /foo/bar /bar -1 test_expect_success 'strip_path_suffix' ' test c:/msysgit = $(test-path-utils strip_path_suffix \ diff --git a/test-path-utils.c b/test-path-utils.c index acb05600df..0092cbf354 100644 --- a/test-path-utils.c +++ b/test-path-utils.c @@ -1,6 +1,33 @@ #include "cache.h" #include "string-list.h" +/* + * A "string_list_each_func_t" function that normalizes an entry from + * GIT_CEILING_DIRECTORIES. If the path is unusable for some reason, + * die with an explanation. + */ +static int normalize_ceiling_entry(struct string_list_item *item, void *unused) +{ + const char *ceil = item->string; + int len = strlen(ceil); + char buf[PATH_MAX+1]; + + if (len == 0) + die("Empty path is not supported"); + if (len > PATH_MAX) + die("Path \"%s\" is too long", ceil); + if (!is_absolute_path(ceil)) + die("Path \"%s\" is not absolute", ceil); + if (normalize_path_copy(buf, ceil) < 0) + die("Path \"%s\" could not be normalized", ceil); + len = strlen(buf); + if (len > 1 && buf[len-1] == '/') + die("Normalized path \"%s\" ended with slash", buf); + free(item->string); + item->string = xstrdup(buf); + return 1; +} + int main(int argc, char **argv) { if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) { @@ -33,10 +60,26 @@ int main(int argc, char **argv) if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) { int len; struct string_list ceiling_dirs = STRING_LIST_INIT_DUP; + char *path = xstrdup(argv[2]); + /* + * We have to normalize the arguments because under + * Windows, bash mangles arguments that look like + * absolute POSIX paths or colon-separate lists of + * absolute POSIX paths into DOS paths (e.g., + * "/foo:/foo/bar" might be converted to + * "D:\Src\msysgit\foo;D:\Src\msysgit\foo\bar"), + * whereas longest_ancestor_length() requires paths + * that use forward slashes. + */ + if (normalize_path_copy(path, path)) + die("Path \"%s\" could not be normalized", argv[2]); string_list_split(&ceiling_dirs, argv[3], PATH_SEP, -1); - len = longest_ancestor_length(argv[2], &ceiling_dirs); + filter_string_list(&ceiling_dirs, 0, + normalize_ceiling_entry, NULL); + len = longest_ancestor_length(path, &ceiling_dirs); string_list_clear(&ceiling_dirs, 0); + free(path); printf("%d\n", len); return 0; } From 1b77d83cab798668d8a54a05b3fa0262486f7dfc Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:26 +0100 Subject: [PATCH 7/8] setup_git_directory_gently_1(): resolve symlinks in ceiling paths longest_ancestor_length() relies on a textual comparison of directory parts to find the part of path that overlaps with one of the paths in prefix_list. But this doesn't work if any of the prefixes involves a symbolic link, because the directories will look different even though they might logically refer to the same directory. So canonicalize the paths listed in GIT_CEILING_DIRECTORIES using real_path_if_valid() before passing them to longest_ancestor_length(). (Also rename normalize_ceiling_entry() to canonicalize_ceiling_entry() to reflect the change.) path is already in canonical form, so doesn't need to be canonicalized again. This fixes some problems with using GIT_CEILING_DIRECTORIES that contains paths involving symlinks, including t4035 if run with --root set to a path involving symlinks. Please note that test t0060 is *not* changed analogously, because that would make the test suite results dependent on the contents of the local root directory. However, real_path() is already tested independently, and the "ancestor" tests cover the non-normalization aspects of longest_ancestor_length(), so coverage remains sufficient. Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- setup.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/setup.c b/setup.c index df97ad3976..f108c4b990 100644 --- a/setup.c +++ b/setup.c @@ -622,24 +622,23 @@ static dev_t get_device_or_die(const char *path, const char *prefix, int prefix_ } /* - * A "string_list_each_func_t" function that normalizes an entry from - * GIT_CEILING_DIRECTORIES or discards it if unusable. + * A "string_list_each_func_t" function that canonicalizes an entry + * from GIT_CEILING_DIRECTORIES using real_path_if_valid(), or + * discards it if unusable. */ -static int normalize_ceiling_entry(struct string_list_item *item, void *unused) +static int canonicalize_ceiling_entry(struct string_list_item *item, + void *unused) { - const char *ceil = item->string; - int len = strlen(ceil); - char buf[PATH_MAX+1]; + char *ceil = item->string; + const char *real_path; - if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil)) + if (!*ceil || !is_absolute_path(ceil)) return 0; - if (normalize_path_copy(buf, ceil) < 0) + real_path = real_path_if_valid(ceil); + if (!real_path) return 0; - len = strlen(buf); - if (len > 1 && buf[len-1] == '/') - buf[--len] = '\0'; free(item->string); - item->string = xstrdup(buf); + item->string = xstrdup(real_path); return 1; } @@ -681,7 +680,8 @@ static const char *setup_git_directory_gently_1(int *nongit_ok) if (env_ceiling_dirs) { string_list_split(&ceiling_dirs, env_ceiling_dirs, PATH_SEP, -1); - filter_string_list(&ceiling_dirs, 0, normalize_ceiling_entry, NULL); + filter_string_list(&ceiling_dirs, 0, + canonicalize_ceiling_entry, NULL); ceil_offset = longest_ancestor_length(cwd, &ceiling_dirs); string_list_clear(&ceiling_dirs, 0); } From 059b37934c611b1b9b735e0310ba282a0c7f5eba Mon Sep 17 00:00:00 2001 From: Michael Haggerty Date: Sun, 28 Oct 2012 17:16:27 +0100 Subject: [PATCH 8/8] string_list_longest_prefix(): remove function This function was added in f103f95b11d087f07c0c48bf784cd9197e18f203 in the erroneous expectation that it would be used in the reimplementation of longest_ancestor_length(). But it turned out to be easier to use a function specialized for comparing path prefixes (i.e., one that knows about slashes and root paths) than to prepare the paths in such a way that a generic string prefix comparison function can be used. So delete string_list_longest_prefix() and its documentation and test cases. Signed-off-by: Michael Haggerty Signed-off-by: Jeff King --- Documentation/technical/api-string-list.txt | 8 ------ string-list.c | 20 -------------- string-list.h | 8 ------ t/t0063-string-list.sh | 30 --------------------- test-string-list.c | 20 -------------- 5 files changed, 86 deletions(-) diff --git a/Documentation/technical/api-string-list.txt b/Documentation/technical/api-string-list.txt index 94d7a2bd99..618400d803 100644 --- a/Documentation/technical/api-string-list.txt +++ b/Documentation/technical/api-string-list.txt @@ -75,14 +75,6 @@ Functions to be deleted. Preserve the order of the items that are retained. -`string_list_longest_prefix`:: - - Return the longest string within a string_list that is a - prefix (in the sense of prefixcmp()) of the specified string, - or NULL if no such prefix exists. This function does not - require the string_list to be sorted (it does a linear - search). - `print_string_list`:: Dump a string_list to stdout, useful mainly for debugging purposes. It diff --git a/string-list.c b/string-list.c index c54b816244..decfa747fc 100644 --- a/string-list.c +++ b/string-list.c @@ -136,26 +136,6 @@ void filter_string_list(struct string_list *list, int free_util, list->nr = dst; } -char *string_list_longest_prefix(const struct string_list *prefixes, - const char *string) -{ - int i, max_len = -1; - char *retval = NULL; - - for (i = 0; i < prefixes->nr; i++) { - char *prefix = prefixes->items[i].string; - if (!prefixcmp(string, prefix)) { - int len = strlen(prefix); - if (len > max_len) { - retval = prefix; - max_len = len; - } - } - } - - return retval; -} - void string_list_clear(struct string_list *list, int free_util) { if (list->items) { diff --git a/string-list.h b/string-list.h index 5efd07b44e..3a6a6dc392 100644 --- a/string-list.h +++ b/string-list.h @@ -38,14 +38,6 @@ int for_each_string_list(struct string_list *list, void filter_string_list(struct string_list *list, int free_util, string_list_each_func_t want, void *cb_data); -/* - * Return the longest string in prefixes that is a prefix (in the - * sense of prefixcmp()) of string, or NULL if no such prefix exists. - * This function does not require the string_list to be sorted (it - * does a linear search). - */ -char *string_list_longest_prefix(const struct string_list *prefixes, const char *string); - /* Use these functions only on sorted lists: */ int string_list_has_string(const struct string_list *list, const char *string); diff --git a/t/t0063-string-list.sh b/t/t0063-string-list.sh index 41c8826a74..dbfc05ebdc 100755 --- a/t/t0063-string-list.sh +++ b/t/t0063-string-list.sh @@ -17,14 +17,6 @@ test_split () { " } -test_longest_prefix () { - test "$(test-string-list longest_prefix "$1" "$2")" = "$3" -} - -test_no_longest_prefix () { - test_must_fail test-string-list longest_prefix "$1" "$2" -} - test_split "foo:bar:baz" ":" "-1" <|- */ - struct string_list prefixes = STRING_LIST_INIT_DUP; - int retval; - const char *prefix_string = argv[2]; - const char *string = argv[3]; - const char *match; - - parse_string_list(&prefixes, prefix_string); - match = string_list_longest_prefix(&prefixes, string); - if (match) { - printf("%s\n", match); - retval = 0; - } - else - retval = 1; - string_list_clear(&prefixes, 0); - return retval; - } - fprintf(stderr, "%s: unknown function name: %s\n", argv[0], argv[1] ? argv[1] : "(there was none)"); return 1;