From d8c06070a87ced23712eaa2b55a0cebb148f9f9a Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Feb 2017 20:21:38 +0100 Subject: [PATCH 1/4] path: extract `win32_path_prefix` function Extract code which determines if a path is at a Windows system's root. This incluses drive prefixes (e.g. "C:\") as well as network computer names (e.g. "//computername/"). --- src/path.c | 56 ++++++++++++++++++++++++++++++++---------------------- 1 file changed, 33 insertions(+), 23 deletions(-) diff --git a/src/path.c b/src/path.c index bffde930b..0e9e04d72 100644 --- a/src/path.c +++ b/src/path.c @@ -110,6 +110,34 @@ Exit: return result; } +/* + * Determine if the path is a Windows prefix and, if so, returns + * its actual lentgh. If it is not a prefix, returns -1. + */ +static int win32_prefix_length(const char *path, int len) +{ +#ifndef GIT_WIN32 + GIT_UNUSED(path); + GIT_UNUSED(len); +#else + /* + * Mimic unix behavior where '/.git' returns '/': 'C:/.git' will return + * 'C:/' here + */ + if (len == 2 && LOOKS_LIKE_DRIVE_PREFIX(path)) + return 3; + + /* + * Similarly checks if we're dealing with a network computer name + * '//computername/.git' will return '//computername/' + */ + if (looks_like_network_computer_name(path, len)) + return len + 1; +#endif + + return -1; +} + /* * Based on the Android implementation, BSD licensed. * Check http://android.git.kernel.org/ @@ -117,7 +145,7 @@ Exit: int git_path_dirname_r(git_buf *buffer, const char *path) { const char *endp; - int result, len; + int len; /* Empty or NULL string gets treated as "." */ if (path == NULL || *path == '\0') { @@ -146,35 +174,17 @@ int git_path_dirname_r(git_buf *buffer, const char *path) endp--; } while (endp > path && *endp == '/'); + if ((len = win32_prefix_length(path, endp - path + 1)) > 0) + goto Exit; + /* Cast is safe because max path < max int */ len = (int)(endp - path + 1); -#ifdef GIT_WIN32 - /* Mimic unix behavior where '/.git' returns '/': 'C:/.git' will return - 'C:/' here */ - - if (len == 2 && LOOKS_LIKE_DRIVE_PREFIX(path)) { - len = 3; - goto Exit; - } - - /* Similarly checks if we're dealing with a network computer name - '//computername/.git' will return '//computername/' */ - - if (looks_like_network_computer_name(path, len)) { - len++; - goto Exit; - } - -#endif - Exit: - result = len; - if (buffer != NULL && git_buf_set(buffer, path, len) < 0) return -1; - return result; + return len; } From 5d59520ccd4816080e68b5a3057653c15f3d6816 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 7 Feb 2017 20:30:11 +0100 Subject: [PATCH 2/4] path: get correct dirname for Windows root Getting the dirname of a filesystem root should return the filesystem root itself. E.g. the dirname of "/" is always "/". On Windows, we emulate this behavior and as such, we should return e.g. "C:/" if calling dirname on "C:/". But we currently fail to do so and instead return ".", as we do not check if we actually have a Windows prefix before stripping off the last directory component. Fix this by calling out to `win32_prefix_length` immediately after stripping trailing slashes, returning early if we have a prefix. --- src/path.c | 3 +++ tests/core/path.c | 2 ++ 2 files changed, 5 insertions(+) diff --git a/src/path.c b/src/path.c index 0e9e04d72..3c78c8b7c 100644 --- a/src/path.c +++ b/src/path.c @@ -159,6 +159,9 @@ int git_path_dirname_r(git_buf *buffer, const char *path) while (endp > path && *endp == '/') endp--; + if ((len = win32_prefix_length(path, endp - path + 1)) > 0) + goto Exit; + /* Find the start of the dir */ while (endp > path && *endp != '/') endp--; diff --git a/tests/core/path.c b/tests/core/path.c index 71c6eda58..eaaaf7245 100644 --- a/tests/core/path.c +++ b/tests/core/path.c @@ -89,8 +89,10 @@ void test_core_path__00_dirname(void) check_dirname(REP16("/abc"), REP15("/abc")); #ifdef GIT_WIN32 + check_dirname("C:/", "C:/"); check_dirname("C:/path/", "C:/"); check_dirname("C:/path", "C:/"); + check_dirname("//computername/", "//computername/"); check_dirname("//computername/path/", "//computername/"); check_dirname("//computername/path", "//computername/"); check_dirname("//computername/sub/path/", "//computername/sub"); From 9e8d75c7d447531d81133bf880c1a5f45a45c340 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 8 Feb 2017 11:41:10 +0100 Subject: [PATCH 3/4] path: ensure dirname on Win32 prefix always has a trailing '/' When calling `git_path_dirname_r` on a Win32 prefix, e.g. a drive or network share prefix, we always want to return the trailing '/'. This does not work currently when passing in a path like 'C:', where the '/' would not be appended correctly. Fix this by appending a '/' if we try to normalize a Win32 prefix and there is no trailing '/'. --- src/path.c | 22 +++++++++++++++------- tests/core/path.c | 2 ++ 2 files changed, 17 insertions(+), 7 deletions(-) diff --git a/src/path.c b/src/path.c index 3c78c8b7c..c3d3eb1ce 100644 --- a/src/path.c +++ b/src/path.c @@ -125,14 +125,14 @@ static int win32_prefix_length(const char *path, int len) * 'C:/' here */ if (len == 2 && LOOKS_LIKE_DRIVE_PREFIX(path)) - return 3; + return 2; /* * Similarly checks if we're dealing with a network computer name * '//computername/.git' will return '//computername/' */ if (looks_like_network_computer_name(path, len)) - return len + 1; + return len; #endif return -1; @@ -145,7 +145,7 @@ static int win32_prefix_length(const char *path, int len) int git_path_dirname_r(git_buf *buffer, const char *path) { const char *endp; - int len; + int is_prefix = 0, len; /* Empty or NULL string gets treated as "." */ if (path == NULL || *path == '\0') { @@ -159,8 +159,10 @@ int git_path_dirname_r(git_buf *buffer, const char *path) while (endp > path && *endp == '/') endp--; - if ((len = win32_prefix_length(path, endp - path + 1)) > 0) + if ((len = win32_prefix_length(path, endp - path + 1)) > 0) { + is_prefix = 1; goto Exit; + } /* Find the start of the dir */ while (endp > path && *endp != '/') @@ -177,15 +179,21 @@ int git_path_dirname_r(git_buf *buffer, const char *path) endp--; } while (endp > path && *endp == '/'); - if ((len = win32_prefix_length(path, endp - path + 1)) > 0) + if ((len = win32_prefix_length(path, endp - path + 1)) > 0) { + is_prefix = 1; goto Exit; + } /* Cast is safe because max path < max int */ len = (int)(endp - path + 1); Exit: - if (buffer != NULL && git_buf_set(buffer, path, len) < 0) - return -1; + if (buffer) { + if (git_buf_set(buffer, path, len) < 0) + return -1; + if (is_prefix && git_buf_putc(buffer, '/') < 0) + return -1; + } return len; } diff --git a/tests/core/path.c b/tests/core/path.c index eaaaf7245..fefe2aeac 100644 --- a/tests/core/path.c +++ b/tests/core/path.c @@ -90,9 +90,11 @@ void test_core_path__00_dirname(void) #ifdef GIT_WIN32 check_dirname("C:/", "C:/"); + check_dirname("C:", "C:/"); check_dirname("C:/path/", "C:/"); check_dirname("C:/path", "C:/"); check_dirname("//computername/", "//computername/"); + check_dirname("//computername", "//computername/"); check_dirname("//computername/path/", "//computername/"); check_dirname("//computername/path", "//computername/"); check_dirname("//computername/sub/path/", "//computername/sub"); From 3428a523597a75127bf258b6d74c2871df5e8f0d Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 8 Feb 2017 12:02:32 +0100 Subject: [PATCH 4/4] tests: repo: assert discovery starting at Win32 roots finishes As of recently, we failed to correctly discover repositories at a Win32 system root. Instead of aborting the upwards-traversal of the file system, we were looping infinitely when traversal started at either a Win32 drive prefix ("C:/") or a network path ("//somehost"). The issue has been fixed, so add a test to catch regressions. --- tests/repo/discover.c | 9 +++++++++ tests/resources/testrepo.git/index | Bin 2 files changed, 9 insertions(+) mode change 100644 => 100755 tests/resources/testrepo.git/index diff --git a/tests/repo/discover.c b/tests/repo/discover.c index 48aa27581..abb7bd12b 100644 --- a/tests/repo/discover.c +++ b/tests/repo/discover.c @@ -199,3 +199,12 @@ void test_repo_discover__discovery_starting_at_file_succeeds(void) ensure_repository_discover(SUB_REPOSITORY_FOLDER "/file", ceiling_dirs.ptr, SUB_REPOSITORY_GITDIR); } + +void test_repo_discover__discovery_starting_at_system_root_causes_no_hang(void) +{ +#ifdef GIT_WIN32 + git_buf out = GIT_BUF_INIT; + cl_git_fail(git_repository_discover(&out, "C:/", 0, NULL)); + cl_git_fail(git_repository_discover(&out, "//localhost/", 0, NULL)); +#endif +} diff --git a/tests/resources/testrepo.git/index b/tests/resources/testrepo.git/index old mode 100644 new mode 100755