[PATCH v6 7/7] RFC: Change error handling scheme in read_gitfile_gently

["Erik Elfström" <erik.elfstrom@gmail.com>]

Signed-off-by: Erik Elfström <erik.elfstrom@gmail.com>
---

Since there was a lot of discussion on error reporting strategy on
the previous patch I have done a quick prototype of the theme
proposed by Jonathan Nieder.

I believe the conclusion was to NOT go this route but this way people
get to see an example of what it could look like to make the
discussion and decision a bit easier.

I will either drop this patch or split it up and squash it into the
appropriate commits (along with change requests if any) depending on
the outcome of the review discussion.

 builtin/clean.c |   3 +-
 cache.h         |   5 +--
 setup.c         | 106 +++++++++++++++++++++++++++++++-------------------------
 3 files changed, 63 insertions(+), 51 deletions(-)

diff --git a/builtin/clean.c b/builtin/clean.c
index d739dcf..7047d6e 100644
--- a/builtin/clean.c
+++ b/builtin/clean.c
@@ -162,7 +162,8 @@ static int is_git_repository(struct strbuf *path)
 	if (path->buf[orig_path_len - 1] != '/')
 		strbuf_addch(path, '/');
 	strbuf_addstr(path, ".git");
-	if (read_gitfile_gently(path->buf, &gitfile_error) || is_git_directory(path->buf))
+	if (read_gitfile_gently(path->buf, &gitfile_error, NULL) ||
+	    is_git_directory(path->buf))
 		ret = 1;
 	if (gitfile_error == READ_GITFILE_ERR_OPEN_FAILED ||
 	    gitfile_error == READ_GITFILE_ERR_READ_FAILED ||
diff --git a/cache.h b/cache.h
index 7c8abcb..76d311a 100644
--- a/cache.h
+++ b/cache.h
@@ -453,8 +453,9 @@ extern const char *get_git_work_tree(void);
 #define READ_GITFILE_ERR_CANT_VERIFY_PATH 7
 #define READ_GITFILE_ERR_NOT_A_REPO 8
 #define READ_GITFILE_ERR_TOO_LARGE 9
-extern const char *read_gitfile_gently(const char *path, int *return_error_code);
-#define read_gitfile(path) read_gitfile_gently((path), NULL)
+extern const char *read_gitfile_gently(const char *path, int *return_err, struct strbuf *err_msg);
+extern const char *read_gitfile(const char *path);
+
 extern const char *resolve_gitdir(const char *suspect);
 extern void set_git_work_tree(const char *tree);
 
diff --git a/setup.c b/setup.c
index bfaf4a6..49274b3 100644
--- a/setup.c
+++ b/setup.c
@@ -374,15 +374,16 @@ static int check_repository_format_gently(const char *gitdir, int *nongit_ok)
  * Try to read the location of the git directory from the .git file,
  * return path to git directory if found.
  *
- * On failure, if return_error_code is not NULL, return_error_code
- * will be set to an error code and NULL will be returned. If
- * return_error_code is NULL the function will die instead (for most
- * cases).
+ * In the event of an error, return_err will be set to an error code
+ * and err_msg will be set to an error message describing the error
+ * and NULL will be returned. If no error reporting is required, pass
+ * NULL for return_err and/or err_msg.
  */
-const char *read_gitfile_gently(const char *path, int *return_error_code)
+const char *read_gitfile_gently(const char *path, int *return_err,
+				struct strbuf *err_msg)
 {
 	static const int one_MB = 1 << 20;
-	int error_code = 0;
+	const char *ret = NULL;
 	char *buf = NULL;
 	char *dir = NULL;
 	const char *slash;
@@ -390,42 +391,59 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 	int fd;
 	ssize_t len;
 	int is_git_dir;
-	struct strbuf err_msg = STRBUF_INIT;
+	int is_git_dir_err;
+
+	if (return_err)
+		*return_err = 0;
 
 	if (stat(path, &st)) {
-		error_code = READ_GITFILE_ERR_STAT_FAILED;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_STAT_FAILED,
+			  "Could not stat: '%s'", path);
 		goto cleanup_return;
 	}
 	if (!S_ISREG(st.st_mode)) {
-		error_code = READ_GITFILE_ERR_NOT_A_FILE;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_NOT_A_FILE,
+			  "Not a file: '%s'", path);
 		goto cleanup_return;
 	}
 	fd = open(path, O_RDONLY);
 	if (fd < 0) {
-		error_code = READ_GITFILE_ERR_OPEN_FAILED;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_OPEN_FAILED,
+			  "Error opening '%s'", path);
 		goto cleanup_return;
 	}
 	if (st.st_size > one_MB) {
 		close(fd);
-		error_code = READ_GITFILE_ERR_TOO_LARGE;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_TOO_LARGE,
+			  "Too large to be a .git file: '%s'", path);
 		goto cleanup_return;
 	}
 	buf = xmalloc(st.st_size + 1);
 	len = read_in_full(fd, buf, st.st_size);
 	close(fd);
 	if (len != st.st_size) {
-		error_code = READ_GITFILE_ERR_READ_FAILED;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_READ_FAILED,
+			  "Error reading %s", path);
 		goto cleanup_return;
 	}
 	buf[len] = '\0';
 	if (!starts_with(buf, "gitdir: ")) {
-		error_code = READ_GITFILE_ERR_INVALID_FORMAT;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_INVALID_FORMAT,
+			  "Invalid gitfile format: %s", path);
 		goto cleanup_return;
 	}
 	while (buf[len - 1] == '\n' || buf[len - 1] == '\r')
 		len--;
 	if (len < 9) {
-		error_code = READ_GITFILE_ERR_NO_PATH;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_NO_PATH,
+			  "No path in gitfile: %s", path);
 		goto cleanup_return;
 	}
 	buf[len] = '\0';
@@ -442,52 +460,44 @@ const char *read_gitfile_gently(const char *path, int *return_error_code)
 		buf = dir;
 	}
 
-	is_git_dir = is_git_directory_gently(dir, &error_code, &err_msg);
-	if (error_code) {
-		error_code = READ_GITFILE_ERR_CANT_VERIFY_PATH;
+	is_git_dir = is_git_directory_gently(dir, &is_git_dir_err, err_msg);
+	if (is_git_dir_err) {
+		if (return_err)
+			*return_err = READ_GITFILE_ERR_CANT_VERIFY_PATH;
 		goto cleanup_return;
 	}
 	if (!is_git_dir) {
-		error_code = READ_GITFILE_ERR_NOT_A_REPO;
+		set_error(return_err, err_msg,
+			  READ_GITFILE_ERR_NOT_A_REPO,
+			  "Not a git repository: %s", dir);
 		goto cleanup_return;
 	}
-	path = real_path(dir);
+	ret = real_path(dir);
 
 cleanup_return:
 	free(buf);
+	return ret;
+}
 
-	if (return_error_code)
-		*return_error_code = error_code;
+const char *read_gitfile(const char *path)
+{
+	int err;
+	const char *ret;
+	struct strbuf err_msg = STRBUF_INIT;
 
-	if (error_code) {
-		if (return_error_code)
-			return NULL;
+	ret = read_gitfile_gently(path, &err, &err_msg);
 
-		switch (error_code) {
-		case READ_GITFILE_ERR_STAT_FAILED:
-		case READ_GITFILE_ERR_NOT_A_FILE:
-			return NULL;
-		case READ_GITFILE_ERR_OPEN_FAILED:
-			die_errno("Error opening '%s'", path);
-		case READ_GITFILE_ERR_TOO_LARGE:
-			die("Too large to be a .git file: '%s'", path);
-		case READ_GITFILE_ERR_READ_FAILED:
-			die("Error reading %s", path);
-		case READ_GITFILE_ERR_INVALID_FORMAT:
-			die("Invalid gitfile format: %s", path);
-		case READ_GITFILE_ERR_NO_PATH:
-			die("No path in gitfile: %s", path);
-		case READ_GITFILE_ERR_CANT_VERIFY_PATH:
-			die("%s", err_msg.buf);
-		case READ_GITFILE_ERR_NOT_A_REPO:
-			die("Not a git repository: %s", dir);
-		default:
-			assert(0);
-		}
+	switch (err) {
+	case 0: /* No need to free err_msg, will only be
+		 * touched in case of error */
+		return ret;
+	case READ_GITFILE_ERR_STAT_FAILED:
+	case READ_GITFILE_ERR_NOT_A_FILE:
+		strbuf_release(&err_msg);
+		return NULL;
+	default:
+		die("%s", err_msg.buf);
 	}
-
-	strbuf_release(&err_msg);
-	return path;
 }
 
 static const char *setup_explicit_git_dir(const char *gitdirenv,
-- 
2.4.0.60.gf7143f7