Re: [PATCH v2] worktree repair: detect relative path in .git file correctly

"Yoichi NAKAYAMA via GitGitGadget" <[email protected]> writes:

> This is because we wrongly use read_gitfile_gently() which always
> returns an absolute path. To fix this, introduce read_gitfile_raw()
> that is almost same as read_gitfile_gently(), but it skips existence
> check of the referenced repository and returns the unmodified path
> read from .git file.

This is more or less what I expected to see, but two function-scope
static variables are worse than one.  At least let us not
proliferate the bad pattern that makes the functions non-reentrant.

The attached patch updates read_gitfile_raw() in your patch to take
a caller-prepared strbuf to store the value read from the '.git'
file, returning the error code as an integer.  Ideally in the far
future, we would probably want to convert read_gitfile_gently() to
follow a similar function signature, but let us leave it as
#leftoverbits, as it has many more existing callers and all of them
would need adjusting.  On the other hand, it is easier to get the API
in read_gitfile_raw() right while it still has only two callers.


 setup.c | 9 +++------
 setup.h | 2 +-
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git c/setup.c w/setup.c
index af7601ff67..052c7d669b 100644
--- c/setup.c
+++ w/setup.c
@@ -996,7 +996,7 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 	return error_code ? NULL : realpath.buf;
 }
 
-const char *read_gitfile_raw(const char *path, int *return_error_code)
+int read_gitfile_raw(struct strbuf *contents, const char *path)
 {
 	const int max_file_size = 1 << 20;  /* 1MB */
 	int error_code = 0;
@@ -1004,7 +1004,6 @@ const char *read_gitfile_raw(const char *path, int *return_error_code)
 	struct stat st;
 	int fd;
 	ssize_t len;
-	static struct strbuf contents = STRBUF_INIT;
 
 	if (stat(path, &st)) {
 		if (errno == ENOENT || errno == ENOTDIR)
@@ -1047,13 +1046,11 @@ const char *read_gitfile_raw(const char *path, int *return_error_code)
 		error_code = READ_GITFILE_ERR_NO_PATH;
 		goto cleanup_return;
 	}
-	strbuf_reset(&contents);
-	strbuf_add(&contents, buf+8, len-8);
+	strbuf_add(contents, buf+8, len-8);
 
 cleanup_return:
-	*return_error_code = error_code;
 	free(buf);
-	return error_code ? NULL : contents.buf;
+	return error_code;
 }
 
 static void apply_gitdir_and_environment(struct repository *repo, const char *path)
diff --git c/setup.h w/setup.h
index 4c2fcbbeda..7394473e95 100644
--- c/setup.h
+++ w/setup.h
@@ -40,7 +40,7 @@ int is_nonbare_repository_dir(struct strbuf *path);
 #define READ_GITFILE_ERR_IS_A_DIR 10
 void read_gitfile_error_die(int error_code, const char *path);
 const char *read_gitfile_gently(const char *path, int *return_error_code);
-const char *read_gitfile_raw(const char *path, int *return_error_code);
+int read_gitfile_raw(struct strbuf *contents, const char *path);
 #define read_gitfile(path) read_gitfile_gently((path), NULL)
 const char *resolve_gitdir_gently(const char *suspect, int *return_error_code);
 #define resolve_gitdir(path) resolve_gitdir_gently((path), NULL)