git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 4/5] Test and fix normalize_path_copy()

From
Johannes Sixt <j6t@kdbg.org>
Date
Feb 7, 2009, 15:08 UTC
Message-ID
<1234019311-6449-5-git-send-email-j6t@kdbg.org>
In-Reply-To
<1234019311-6449-4-git-send-email-j6t@kdbg.org>

This changes the test-path-utils utility to invoke normalize_path_copy() instead of normalize_absolute_path() because the latter is about to be removed.

The test cases in t0060 are adjusted in two regards:
- normalize_path_copy() more often leaves a trailing slash in the result.
  This has no negative side effects because the new user of this function,
  longest_ancester_length(), already accounts for this behavior.
- The function can fail.

The tests uncover a flaw in normalize_path_copy(): If there are sufficiently many '..' path components so that the root is reached, such as in "/d1/s1/../../d2", then the leading slash was lost. This manifested itself that (assuming there is a repository at /tmp/foo)

  $ git add /d1/../tmp/foo/some-file
reported 'pathspec is outside repository'. This is now fixed.

Moreover, the test case descriptions of t0060 now include the test data and expected outcome.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 path.c                |   16 +++++-----------
 t/t0060-path-utils.sh |   33 +++++++++++++++++----------------
 test-path-utils.c     |    7 ++++---
 3 files changed, 26 insertions(+), 30 deletions(-)
diff --git a/path.c b/path.c
index dc3807a..16628b2 100644
--- a/path.c
+++ b/path.c
@@ -484,18 +484,12 @@ int normalize_path_copy(char *dst, const char *src)
 		 * dst0..dst is prefix portion, and dst[-1] is '/';
 		 * go up one level.
 		 */
-		dst -= 2; /* go past trailing '/' if any */
-		if (dst < dst0)
+		dst--;	/* go to trailing '/' */
+		if (dst <= dst0)
 			return -1;
-		while (1) {
-			if (dst <= dst0)
-				break;
-			c = *dst--;
-			if (c == '/') {	/* MinGW: cannot be '\\' anymore */
-				dst += 2;
-				break;
-			}
-		}
+		/* Windows: dst[-1] cannot be backslash anymore */
+		while (dst0 < dst && dst[-1] != '/')
+			dst--;
 	}
 	*dst = '\0';
 	return 0;
diff --git a/t/t0060-path-utils.sh b/t/t0060-path-utils.sh
index 6e7501f..4ed1f0b 100755
--- a/t/t0060-path-utils.sh
+++ b/t/t0060-path-utils.sh
@@ -8,36 +8,37 @@ test_description='Test various path utilities'
 . ./test-lib.sh
 
 norm_abs() {
-	test_expect_success "normalize absolute" \
-	"test \$(test-path-utils normalize_absolute_path '$1') = '$2'"
+	test_expect_success "normalize absolute: $1 => $2" \
+	"test \"\$(test-path-utils normalize_path_copy '$1')\" = '$2'"
 }
 
 ancestor() {
-	test_expect_success "longest ancestor" \
-	"test \$(test-path-utils longest_ancestor_length '$1' '$2') = '$3'"
+	test_expect_success "longest ancestor: $1 $2 => $3" \
+	"test \"\$(test-path-utils longest_ancestor_length '$1' '$2')\" = '$3'"
 }
 
-norm_abs "" /
+norm_abs "" ""
 norm_abs / /
 norm_abs // /
 norm_abs /// /
 norm_abs /. /
 norm_abs /./ /
-norm_abs /./.. /
-norm_abs /../. /
-norm_abs /./../.// /
+norm_abs /./.. ++failed++
+norm_abs /../. ++failed++
+norm_abs /./../.// ++failed++
 norm_abs /dir/.. /
 norm_abs /dir/sub/../.. /
+norm_abs /dir/sub/../../.. ++failed++
 norm_abs /dir /dir
-norm_abs /dir// /dir
+norm_abs /dir// /dir/
 norm_abs /./dir /dir
-norm_abs /dir/. /dir
-norm_abs /dir///./ /dir
-norm_abs /dir//sub/.. /dir
-norm_abs /dir/sub/../ /dir
-norm_abs //dir/sub/../. /dir
-norm_abs /dir/s1/../s2/ /dir/s2
-norm_abs /d1/s1///s2/..//../s3/ /d1/s3
+norm_abs /dir/. /dir/
+norm_abs /dir///./ /dir/
+norm_abs /dir//sub/.. /dir/
+norm_abs /dir/sub/../ /dir/
+norm_abs //dir/sub/../. /dir/
+norm_abs /dir/s1/../s2/ /dir/s2/
+norm_abs /d1/s1///s2/..//../s3/ /d1/s3/
 norm_abs /d1/s1//../s2/../../d2 /d2
 norm_abs /d1/.../d2 /d1/.../d2
 norm_abs /d1/..././../d2 /d1/d2
diff --git a/test-path-utils.c b/test-path-utils.c
index 7e6fc8d..5168a8e 100644
--- a/test-path-utils.c
+++ b/test-path-utils.c
@@ -2,10 +2,11 @@
 
 int main(int argc, char **argv)
 {
-	if (argc == 3 && !strcmp(argv[1], "normalize_absolute_path")) {
+	if (argc == 3 && !strcmp(argv[1], "normalize_path_copy")) {
 		char *buf = xmalloc(PATH_MAX + 1);
-		int rv = normalize_absolute_path(buf, argv[2]);
-		assert(strlen(buf) == rv);
+		int rv = normalize_path_copy(buf, argv[2]);
+		if (rv)
+			buf = "++failed++";
 		puts(buf);
 		return 0;
 	}
-- 
1.6.1.297.g9b01e
Previous: Johannes SixtNext: Johannes Sixt
Message 20 of 26 in “fix crash in path.c on Windows”
  1. fix crash in path.c on WindowsRené Scharfe, Feb 4, 2009
  2. Junio C HamanoFeb 4, 2009
  3. Johannes SixtFeb 5, 2009
  4. René ScharfeFeb 5, 2009
  5. Johannes SixtFeb 5, 2009
  6. Robin RosenbergFeb 5, 2009
  7. fix t1504 on WindowsRené Scharfe, Feb 5, 2009
  8. Johannes SixtFeb 6, 2009
  9. Johannes SchindelinFeb 6, 2009
  10. Johannes SixtFeb 6, 2009
  11. Johannes SchindelinFeb 6, 2009
  12. Johannes SixtFeb 6, 2009
  13. René ScharfeFeb 6, 2009
  14. Johannes SixtFeb 6, 2009
  15. René ScharfeFeb 6, 2009
  16. 0/5 Consolidate path normalization functionsJohannes Sixt, Feb 7, 2009
  17. 1/5 Make test-path-utils more robust against incorrect useJohannes Sixt, Feb 7, 2009
  18. 2/5 Move sanitary_path_copy() to path.c and rename it to normalize_path_copy()Johannes Sixt, Feb 7, 2009
  19. 3/5 Fix GIT_CEILING_DIRECTORIES on WindowsJohannes Sixt, Feb 7, 2009
  20. 4/5 Test and fix normalize_path_copy()Johannes Sixt, Feb 7, 2009
  21. 5/5 Remove unused normalize_absolute_path()Johannes Sixt, Feb 7, 2009
  22. Robin RosenbergFeb 8, 2009
  23. Johannes SixtFeb 8, 2009
  24. René ScharfeFeb 8, 2009
  25. Johannes SixtFeb 8, 2009
  26. René ScharfeFeb 7, 2009

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.