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

Re: [PATCH v2] Add support for GIT_CEILING_DIRS

From
Johannes Sixt <j.sixt@viscovery.net>
Date
May 15, 2008, 08:38 UTC
Message-ID
<482BF69C.6020604@viscovery.net>
In-Reply-To
<482BE238.5020309@facebook.com>
David Reiss schrieb:
Show 12 quoted lines
>> 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("..");
 	}
Previous: David ReissNext: Johannes Schindelin
Message 4 of 7 in “Add support for GIT_CEILING_DIRS”
  1. Add support for GIT_CEILING_DIRSDavid Reiss, May 15, 2008
  2. Johannes SixtMay 15, 2008
  3. David ReissMay 15, 2008
  4. Johannes SixtMay 15, 2008
  5. Johannes SchindelinMay 15, 2008
  6. David ReissMay 15, 2008
  7. Johannes SchindelinMay 15, 2008

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.