Re: [PATCH v2] Add support for GIT_CEILING_DIRS

[Johannes Sixt <j.sixt@xxxxxxxxxxxxx>]

David Reiss schrieb:
>> If you make it so that the default value of ceil_offset is 0 (i.e. in the
>> absence of any GIT_CEILING_DIRS),
> This is what the new version of the patch does.
> 
>> and at this place you did
>>
>>                 } while (offset > ceil_offset && cwd[--offset] != '/');
>>
>> you wouldn't have to bend backwards with this off-by-one magic, would you?
> It seems like that would cause it to continue on with the outer loop, rather
> than aborting (which is what the current version does) when you hit the ceiling.
> Or maybe I'm misunderstanding something.

Ok, I see. I'm proposing that the meaning of ceil_offset should be "do not look
at this many characters at the beginning of cwd". And we always have
cwd[ceil_offset] == '/'. Something like this on top of your patch.
Further tweaking will be necessary (I'm just illustrating my point),
in particular you could name it min_offset again.

-- Hannes

diff --git a/setup.c b/setup.c
index de9e7f1..5dbc080 100644
--- a/setup.c
+++ b/setup.c
@@ -368,13 +368,13 @@ const char *read_gitfile_gently(const char *path)
 static int longest_ancestor_length(const char *path, const char *prefix_list)
 {
 	const char *ceil, *colon;
-	int max_len = -1;
+	int max_len = 0;

 	if (prefix_list == NULL)
-		return -1;
-	/* "/" is a tricky edge case.  It should always return -1, though. */
+		return 0;
+	/* "/" is a tricky edge case.  It should always return 0, though. */
 	if (!strcmp(path, "/"))
-		return -1;
+		return 0;

 	ceil = prefix_list;
 	for (;;) {
@@ -522,7 +522,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
 	 * Putting it so far to the right is necessary in order to bail out of the
 	 * "--offset" loop early enough.
 	 */
-	ceil_offset = 1 + longest_ancestor_length(cwd, env_ceiling_dirs);
+	ceil_offset = longest_ancestor_length(cwd, env_ceiling_dirs);

 	/*
 	 * Test in the following order (relative to the cwd):
@@ -553,17 +553,16 @@ const char *setup_git_directory_gently(int *nongit_ok)
 			check_repository_format_gently(nongit_ok);
 			return NULL;
 		}
-		do {
-			if (offset <= ceil_offset) {
-				if (nongit_ok) {
-					if (chdir(cwd))
-						die("Cannot come back to cwd");
-					*nongit_ok = 1;
-					return NULL;
-				}
-				die("Not a git repository");
+		do { } while (offset > ceil_offset && cwd[--offset] != '/');
+		if (offset <= ceil_offset) {
+			if (nongit_ok) {
+				if (chdir(cwd))
+					die("Cannot come back to cwd");
+				*nongit_ok = 1;
+				return NULL;
 			}
-		} while (cwd[--offset] != '/');
+			die("Not a git repository");
+		}
 		chdir("..");
 	}


--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html