Merge branch 'js/maint-1.6.0-path-normalize'

* js/maint-1.6.0-path-normalize:
  Remove unused normalize_absolute_path()
  Test and fix normalize_path_copy()
  Fix GIT_CEILING_DIRECTORIES on Windows
  Move sanitary_path_copy() to path.c and rename it to normalize_path_copy()
  Make test-path-utils more robust against incorrect use

cache.h

@@ -627,7 +627,7 @@ int is_directory(const char *);
 const char *make_absolute_path(const char *path);
 const char *make_nonrelative_path(const char *path);
 const char *make_relative_path(const char *abs, const char *base);
-int normalize_absolute_path(char *buf, const char *path);
+int normalize_path_copy(char *dst, const char *src);
 int longest_ancestor_length(const char *path, const char *prefix_list);
 
 /* Read and unpack a sha1 file into memory, write memory to a sha1 file */

path.c

@@ -363,56 +363,97 @@ const char *make_relative_path(const char *abs, const char *base)
 }
 
 /*
- * path = absolute path
+ * It is okay if dst == src, but they should not overlap otherwise.
- * buf = buffer of at least max(2, strlen(path)+1) bytes
- * It is okay if buf == path, but they should not overlap otherwise.
  *
- * Performs the following normalizations on path, storing the result in buf:
+ * Performs the following normalizations on src, storing the result in dst:
- * - Removes trailing slashes.
+ * - Ensures that components are separated by '/' (Windows only)
- * - Removes empty components.
+ * - Squashes sequences of '/'.
  * - Removes "." components.
  * - Removes ".." components, and the components the precede them.
- * "" and paths that contain only slashes are normalized to "/".
+ * Returns failure (non-zero) if a ".." component appears as first path
- * Returns the length of the output.
+ * component anytime during the normalization. Otherwise, returns success (0).
  *
  * Note that this function is purely textual.  It does not follow symlinks,
  * verify the existence of the path, or make any system calls.
  */
-int normalize_absolute_path(char *buf, const char *path)
+int normalize_path_copy(char *dst, const char *src)
 {
-	const char *comp_start = path, *comp_end = path;
+	char *dst0;
-	char *dst = buf;
-	int comp_len;
-	assert(buf);
-	assert(path);
 
-	while (*comp_start) {
+	if (has_dos_drive_prefix(src)) {
-		assert(*comp_start == '/');
+		*dst++ = *src++;
-		while (*++comp_end && *comp_end != '/')
+		*dst++ = *src++;
-			; /* nothing */
-		comp_len = comp_end - comp_start;
-
-		if (!strncmp("/",  comp_start, comp_len) ||
-		    !strncmp("/.", comp_start, comp_len))
-			goto next;
-
-		if (!strncmp("/..", comp_start, comp_len)) {
-			while (dst > buf && *--dst != '/')
-				; /* nothing */
-			goto next;
 	}
+	dst0 = dst;
 
-		memmove(dst, comp_start, comp_len);
+	if (is_dir_sep(*src)) {
-		dst += comp_len;
-	next:
-		comp_start = comp_end;
-	}
-
-	if (dst == buf)
 		*dst++ = '/';
+		while (is_dir_sep(*src))
+			src++;
+	}
 
+	for (;;) {
+		char c = *src;
+
+		/*
+		 * A path component that begins with . could be
+		 * special:
+		 * (1) "." and ends   -- ignore and terminate.
+		 * (2) "./"           -- ignore them, eat slash and continue.
+		 * (3) ".." and ends  -- strip one and terminate.
+		 * (4) "../"          -- strip one, eat slash and continue.
+		 */
+		if (c == '.') {
+			if (!src[1]) {
+				/* (1) */
+				src++;
+			} else if (is_dir_sep(src[1])) {
+				/* (2) */
+				src += 2;
+				while (is_dir_sep(*src))
+					src++;
+				continue;
+			} else if (src[1] == '.') {
+				if (!src[2]) {
+					/* (3) */
+					src += 2;
+					goto up_one;
+				} else if (is_dir_sep(src[2])) {
+					/* (4) */
+					src += 3;
+					while (is_dir_sep(*src))
+						src++;
+					goto up_one;
+				}
+			}
+		}
+
+		/* copy up to the next '/', and eat all '/' */
+		while ((c = *src++) != '\0' && !is_dir_sep(c))
+			*dst++ = c;
+		if (is_dir_sep(c)) {
+			*dst++ = '/';
+			while (is_dir_sep(c))
+				c = *src++;
+			src--;
+		} else if (!c)
+			break;
+		continue;
+
+	up_one:
+		/*
+		 * dst0..dst is prefix portion, and dst[-1] is '/';
+		 * go up one level.
+		 */
+		dst--;	/* go to trailing '/' */
+		if (dst <= dst0)
+			return -1;
+		/* Windows: dst[-1] cannot be backslash anymore */
+		while (dst0 < dst && dst[-1] != '/')
+			dst--;
+	}
 	*dst = '\0';
-	return dst - buf;
+	return 0;
 }
 
 /*
@@ -438,15 +479,16 @@ int longest_ancestor_length(const char *path, const char *prefix_list)
 		return -1;
 
 	for (colon = ceil = prefix_list; *colon; ceil = colon+1) {
-		for (colon = ceil; *colon && *colon != ':'; colon++);
+		for (colon = ceil; *colon && *colon != PATH_SEP; colon++);
 		len = colon - ceil;
 		if (len == 0 || len > PATH_MAX || !is_absolute_path(ceil))
 			continue;
 		strlcpy(buf, ceil, len+1);
-		len = normalize_absolute_path(buf, buf);
+		if (normalize_path_copy(buf, buf) < 0)
-		/* Strip "trailing slashes" from "/". */
+			continue;
-		if (len == 1)
+		len = strlen(buf);
-			len = 0;
+		if (len > 0 && buf[len-1] == '/')
+			buf[--len] = '\0';
 
 		if (!strncmp(path, buf, len) &&
 		    path[len] == '/' &&

setup.c

@@ -4,92 +4,6 @@
 static int inside_git_dir = -1;
 static int inside_work_tree = -1;
 
-static int sanitary_path_copy(char *dst, const char *src)
-{
-	char *dst0;
-
-	if (has_dos_drive_prefix(src)) {
-		*dst++ = *src++;
-		*dst++ = *src++;
-	}
-	dst0 = dst;
-
-	if (is_dir_sep(*src)) {
-		*dst++ = '/';
-		while (is_dir_sep(*src))
-			src++;
-	}
-
-	for (;;) {
-		char c = *src;
-
-		/*
-		 * A path component that begins with . could be
-		 * special:
-		 * (1) "." and ends   -- ignore and terminate.
-		 * (2) "./"           -- ignore them, eat slash and continue.
-		 * (3) ".." and ends  -- strip one and terminate.
-		 * (4) "../"          -- strip one, eat slash and continue.
-		 */
-		if (c == '.') {
-			if (!src[1]) {
-				/* (1) */
-				src++;
-			} else if (is_dir_sep(src[1])) {
-				/* (2) */
-				src += 2;
-				while (is_dir_sep(*src))
-					src++;
-				continue;
-			} else if (src[1] == '.') {
-				if (!src[2]) {
-					/* (3) */
-					src += 2;
-					goto up_one;
-				} else if (is_dir_sep(src[2])) {
-					/* (4) */
-					src += 3;
-					while (is_dir_sep(*src))
-						src++;
-					goto up_one;
-				}
-			}
-		}
-
-		/* copy up to the next '/', and eat all '/' */
-		while ((c = *src++) != '\0' && !is_dir_sep(c))
-			*dst++ = c;
-		if (is_dir_sep(c)) {
-			*dst++ = '/';
-			while (is_dir_sep(c))
-				c = *src++;
-			src--;
-		} else if (!c)
-			break;
-		continue;
-
-	up_one:
-		/*
-		 * dst0..dst is prefix portion, and dst[-1] is '/';
-		 * go up one level.
-		 */
-		dst -= 2; /* go past trailing '/' if any */
-		if (dst < dst0)
-			return -1;
-		while (1) {
-			if (dst <= dst0)
-				break;
-			c = *dst--;
-			if (c == '/') {	/* MinGW: cannot be '\\' anymore */
-				dst += 2;
-				break;
-			}
-		}
-	}
-	*dst = '\0';
-	return 0;
-}
-
 const char *prefix_path(const char *prefix, int len, const char *path)
 {
 	const char *orig = path;
@@ -101,7 +15,7 @@ const char *prefix_path(const char *prefix, int len, const char *path)
 			memcpy(sanitized, prefix, len);
 		strcpy(sanitized + len, path);
 	}
-	if (sanitary_path_copy(sanitized, sanitized))
+	if (normalize_path_copy(sanitized, sanitized))
 		goto error_out;
 	if (is_absolute_path(orig)) {
 		const char *work_tree = get_git_work_tree();

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_expect_success "normalize absolute: $1 => $2" \
-	"test \$(test-path-utils normalize_absolute_path '$1') = '$2'"
+	"test \"\$(test-path-utils normalize_path_copy '$1')\" = '$2'"
 }
 
 ancestor() {
-	test_expect_success "longest ancestor" \
+	test_expect_success "longest ancestor: $1 $2 => $3" \
-	"test \$(test-path-utils longest_ancestor_length '$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 /./.. ++failed++
-norm_abs /../. /
+norm_abs /../. ++failed++
-norm_abs /./../.// /
+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///./ /dir
+norm_abs /dir///./ /dir/
-norm_abs /dir//sub/.. /dir
+norm_abs /dir//sub/.. /dir/
-norm_abs /dir/sub/../ /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 /dir/s1/../s2/ /dir/s2/
-norm_abs /d1/s1///s2/..//../s3/ /d1/s3
+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

t/t1504-ceiling-dirs.sh

@@ -93,13 +93,13 @@ GIT_CEILING_DIRECTORIES="$TRASH_ROOT/subdi"
 test_prefix subdir_ceil_at_subdi_slash "sub/dir/"
 
 
-GIT_CEILING_DIRECTORIES="foo:$TRASH_ROOT/sub"
+GIT_CEILING_DIRECTORIES="/foo:$TRASH_ROOT/sub"
 test_fail second_of_two
 
-GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub:bar"
+GIT_CEILING_DIRECTORIES="$TRASH_ROOT/sub:/bar"
 test_fail first_of_two
 
-GIT_CEILING_DIRECTORIES="foo:$TRASH_ROOT/sub:bar"
+GIT_CEILING_DIRECTORIES="/foo:$TRASH_ROOT/sub:/bar"
 test_fail second_of_three
 
 

test-path-utils.c

@@ -2,11 +2,13 @@
 
 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]);
+		int rv = normalize_path_copy(buf, argv[2]);
-		assert(strlen(buf) == rv);
+		if (rv)
+			buf = "++failed++";
 		puts(buf);
+		return 0;
 	}
 
 	if (argc >= 2 && !strcmp(argv[1], "make_absolute_path")) {
@@ -15,12 +17,16 @@ int main(int argc, char **argv)
 			argc--;
 			argv++;
 		}
+		return 0;
 	}
 
 	if (argc == 4 && !strcmp(argv[1], "longest_ancestor_length")) {
 		int len = longest_ancestor_length(argv[2], argv[3]);
 		printf("%d\n", len);
+		return 0;
 	}
 
-	return 0;
+	fprintf(stderr, "%s: unknown function name: %s\n", argv[0],
+		argv[1] ? argv[1] : "(there was none)");
+	return 1;
 }