git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j6t@kdbg.org>
To: rene.scharfe@lsrfire.ath.cx
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH 4/5] Test and fix normalize_path_copy()
Date: Sat,  7 Feb 2009 16:08:30 +0100	[thread overview]
Message-ID: <1234019311-6449-5-git-send-email-j6t@kdbg.org> (raw)
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

  reply	other threads:[~2009-02-07 15:17 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-02-04 23:00 [PATCH] fix crash in path.c on Windows René Scharfe
2009-02-04 23:51 ` Junio C Hamano
2009-02-05  7:57 ` Johannes Sixt
2009-02-05 16:48   ` René Scharfe
2009-02-05 17:13     ` Johannes Sixt
2009-02-05 20:41     ` Robin Rosenberg
2009-02-05 19:35 ` [PATCH] fix t1504 " René Scharfe
2009-02-06 12:55   ` Johannes Sixt
2009-02-06 13:11     ` Johannes Schindelin
2009-02-06 13:17       ` Johannes Sixt
2009-02-06 13:26         ` Johannes Schindelin
2009-02-06 13:36           ` Johannes Sixt
2009-02-06 17:18     ` René Scharfe
2009-02-06 19:23       ` Johannes Sixt
2009-02-06 21:45         ` René Scharfe
2009-02-07 15:08           ` [PATCH 0/5] Consolidate path normalization functions Johannes Sixt
2009-02-07 15:08             ` [PATCH 1/5] Make test-path-utils more robust against incorrect use Johannes Sixt
2009-02-07 15:08               ` [PATCH 2/5] Move sanitary_path_copy() to path.c and rename it to normalize_path_copy() Johannes Sixt
2009-02-07 15:08                 ` [PATCH 3/5] Fix GIT_CEILING_DIRECTORIES on Windows Johannes Sixt
2009-02-07 15:08                   ` Johannes Sixt [this message]
2009-02-07 15:08                     ` [PATCH 5/5] Remove unused normalize_absolute_path() Johannes Sixt
2009-02-08  0:08                     ` [PATCH 4/5] Test and fix normalize_path_copy() Robin Rosenberg
2009-02-08  8:52                       ` Johannes Sixt
2009-02-08 14:46                     ` René Scharfe
2009-02-08 15:50                       ` Johannes Sixt
2009-02-07  0:25     ` [PATCH] fix t1504 on Windows René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1234019311-6449-5-git-send-email-j6t@kdbg.org \
    --to=j6t@kdbg.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=rene.scharfe@lsrfire.ath.cx \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).