[PATCH 1/2 v2] setup: return correct prefix if worktree is '/'

[PATCH 1/2 v2] setup: return correct prefix if worktree is '/'

[Nguyễn Thái Ngọc Duy]

The same old problem reappears after setup code is reworked.  We tend
to assume there is at least one path component in a path and forget
that path can be simply '/'.

Reported-by: Matthijs Kooijman <matthijs@stdin.nl>
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 It just looks better than previous version.

 dir.c   |   31 +++++++++++++++++++++++++++++++
 dir.h   |    1 +
 setup.c |    7 ++++---
 3 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/dir.c b/dir.c
index 325fb56..6c49a73 100644
--- a/dir.c
+++ b/dir.c
@@ -1152,6 +1152,37 @@ char *get_relative_cwd(char *buffer, int size, const char *dir)
 	}
 }
 
+ * Given two normalized paths (a trailing slash is ok), if subdir is
+ * outside dir, return -1.  Otherwise return the offset in subdir that
+ * can be used as relative path to dir.
+ */
+int dir_inside_of(const char *subdir, const char *dir)
+{
+	int offset = 0;
+
+	assert(dir && subdir && *dir && *subdir);
+
+	while (*dir && *subdir && *dir == *subdir) {
+		dir++;
+		subdir++;
+		offset++;
+	}
+
+	/* hel[p]/me vs hel[l]/yeah */
+	if (*dir && *subdir)
+		return -1;
+
+	if (!*subdir)
+		return !*dir ? offset : -1; /* same dir */
+
+	/* foo/[b]ar vs foo/[] */
+	if (is_dir_sep(dir[-1]))
+		return is_dir_sep(subdir[-1]) ? offset : -1;
+
+	/* foo[/]bar vs foo[] */
+	return is_dir_sep(*subdir) ? offset + 1 : -1;
+}
+
 int is_inside_dir(const char *dir)
 {
 	char buffer[PATH_MAX];
diff --git a/dir.h b/dir.h
index aa511da..83e2992 100644
--- a/dir.h
+++ b/dir.h
@@ -87,6 +87,7 @@ extern int file_exists(const char *);
 
 extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
+extern int dir_inside_of(const char *subdir, const char *dir);
 
 static inline int is_dot_or_dotdot(const char *name)
 {
diff --git a/setup.c b/setup.c
index 03cd84f..b6e6b5a 100644
--- a/setup.c
+++ b/setup.c
@@ -325,6 +325,7 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
 	const char *work_tree_env = getenv(GIT_WORK_TREE_ENVIRONMENT);
 	const char *worktree;
 	char *gitfile;
+	int offset;
 
 	if (PATH_MAX - 40 < strlen(gitdirenv))
 		die("'$%s' too big", GIT_DIR_ENVIRONMENT);
@@ -390,15 +391,15 @@ static const char *setup_explicit_git_dir(const char *gitdirenv,
 		return NULL;
 	}
 
-	if (!prefixcmp(cwd, worktree) &&
-	    cwd[strlen(worktree)] == '/') { /* cwd inside worktree */
+	offset = dir_inside_of(cwd, worktree);
+	if (offset >= 0) {	/* cwd inside worktree? */
 		set_git_dir(real_path(gitdirenv));
 		if (chdir(worktree))
 			die_errno("Could not chdir to '%s'", worktree);
 		cwd[len++] = '/';
 		cwd[len] = '\0';
 		free(gitfile);
-		return cwd + strlen(worktree) + 1;
+		return cwd + offset;
 	}
 
 	/* cwd outside worktree */
-- 
1.7.4.74.g639db

[PATCH 2/2] Kill off get_relative_cwd()

[Nguyễn Thái Ngọc Duy]

Function dir_inside_of() does something similar (correctly), but looks
easier to understand and does not bundle cwd to its business. Given
get_relative_cwd's only user is is_inside_dir, we can kill it for
good.

Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
 2011/3/25 Michael J Gruber <git@drmicha.warpmail.net>:
 > get_relative_cwd() tries to determine a common prefix for dir and
 > cwd.
 > The fix in
 > 490544b (get_cwd_relative(): do not misinterpret suffix as
 > subdirectory, 2010-05-22)
 > made the logic less naive (so that foo-bar is not misdetected as
 > being
 > within foo) but broke some other cases, in particular foo not being
 > detected as being within foo/ any more.
 >
 > Fix it by taking into a account that a directory name may or may
 > not end
 > in /.

 OK I stared at the code long enough and have come to the conclusion
 that your fix is essentially fbbb4e1 (get_cwd_relative(): do not
 misinterpret root path - 2010-11-20) but the structure is similar to
 my old is_subdir_or_same() (now dir_inside_of). So let's kill that
 cryptic switch.

 dir.c |   55 ++++++-------------------------------------------------
 dir.h |    1 -
 2 files changed, 6 insertions(+), 50 deletions(-)

diff --git a/dir.c b/dir.c
index 6c49a73..bd9e989 100644
--- a/dir.c
+++ b/dir.c
@@ -1105,53 +1105,6 @@ int file_exists(const char *f)
 }
 
 /*
- * get_relative_cwd() gets the prefix of the current working directory
- * relative to 'dir'.  If we are not inside 'dir', it returns NULL.
- *
- * As a convenience, it also returns NULL if 'dir' is already NULL.  The
- * reason for this behaviour is that it is natural for functions returning
- * directory names to return NULL to say "this directory does not exist"
- * or "this directory is invalid".  These cases are usually handled the
- * same as if the cwd is not inside 'dir' at all, so get_relative_cwd()
- * returns NULL for both of them.
- *
- * Most notably, get_relative_cwd(buffer, size, get_git_work_tree())
- * unifies the handling of "outside work tree" with "no work tree at all".
- */
-char *get_relative_cwd(char *buffer, int size, const char *dir)
-{
-	char *cwd = buffer;
-
-	if (!dir)
-		return NULL;
-	if (!getcwd(buffer, size))
-		die_errno("can't find the current directory");
-
-	if (!is_absolute_path(dir))
-		dir = real_path(dir);
-
-	while (*dir && *dir == *cwd) {
-		dir++;
-		cwd++;
-	}
-	if (*dir)
-		return NULL;
-	switch (*cwd) {
-	case '\0':
-		return cwd;
-	case '/':
-		return cwd + 1;
-	default:
-		/*
-		 * dir can end with a path separator when it's root
-		 * directory. Return proper prefix in that case.
-		 */
-		if (dir[-1] == '/')
-			return cwd;
-		return NULL;
-	}
-}
-
  * Given two normalized paths (a trailing slash is ok), if subdir is
  * outside dir, return -1.  Otherwise return the offset in subdir that
  * can be used as relative path to dir.
@@ -1185,8 +1138,12 @@ int dir_inside_of(const char *subdir, const char *dir)
 
 int is_inside_dir(const char *dir)
 {
-	char buffer[PATH_MAX];
-	return get_relative_cwd(buffer, sizeof(buffer), dir) != NULL;
+	char cwd[PATH_MAX];
+	if (!dir)
+		return 0;
+	if (!getcwd(cwd, sizeof(cwd)))
+		die_errno("can't find the current directory");
+	return dir_inside_of(cwd, dir) >= 0;
 }
 
 int is_empty_dir(const char *path)
diff --git a/dir.h b/dir.h
index 83e2992..433b5b4 100644
--- a/dir.h
+++ b/dir.h
@@ -85,7 +85,6 @@ extern void add_exclude(const char *string, const char *base,
 extern void free_excludes(struct exclude_list *el);
 extern int file_exists(const char *);
 
-extern char *get_relative_cwd(char *buffer, int size, const char *dir);
 extern int is_inside_dir(const char *dir);
 extern int dir_inside_of(const char *subdir, const char *dir);
 
-- 
1.7.4.74.g639db