Commit 2b490284959998167c095433693e9e7e66d25d6c
2016-06-24T15:59:37
find_repo: Clean up and simplify logic find_repo had a complex loop and heavily nested conditionals, making it difficult to follow. Simplify this as much as possible: - Separate assignments from conditionals. - Check the complex loop condition in the only place it can change. - Break out of the loop on error, rather than going through the rest of the loop body first. - Handle error cases by immediately breaking, rather than nesting conditionals. - Free repo_link unconditionally on the way out of the function, rather than in multiple places. - Add more comments on the remaining complex steps.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118
diff --git a/src/repository.c b/src/repository.c
index 635b13b..ecc0780 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -357,6 +357,7 @@ static int find_repo(
{
int error;
git_buf path = GIT_BUF_INIT;
+ git_buf repo_link = GIT_BUF_INIT;
struct stat st;
dev_t initial_device = 0;
int min_iterations;
@@ -365,7 +366,8 @@ static int find_repo(
git_buf_free(repo_path);
- if ((error = git_path_prettify(&path, start_path, NULL)) < 0)
+ error = git_path_prettify(&path, start_path, NULL);
+ if (error < 0)
return error;
/* in_dot_git toggles each loop:
@@ -383,12 +385,13 @@ static int find_repo(
min_iterations = 2;
}
- while (!error && (min_iterations || !(path.ptr[ceiling_offset] == 0 ||
- (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))) {
+ for (;;) {
if (!(flags & GIT_REPOSITORY_OPEN_NO_DOTGIT)) {
- if (!in_dot_git)
- if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0)
+ if (!in_dot_git) {
+ error = git_buf_joinpath(&path, path.ptr, DOT_GIT);
+ if (error < 0)
break;
+ }
in_dot_git = !in_dot_git;
}
@@ -397,7 +400,7 @@ static int find_repo(
if (initial_device == 0)
initial_device = st.st_dev;
else if (st.st_dev != initial_device &&
- (flags & GIT_REPOSITORY_OPEN_CROSS_FS) == 0)
+ !(flags & GIT_REPOSITORY_OPEN_CROSS_FS))
break;
if (S_ISDIR(st.st_mode)) {
@@ -408,25 +411,22 @@ static int find_repo(
}
}
else if (S_ISREG(st.st_mode)) {
- git_buf repo_link = GIT_BUF_INIT;
-
- if (!(error = read_gitfile(&repo_link, path.ptr))) {
- if (valid_repository_path(&repo_link)) {
- git_buf_swap(repo_path, &repo_link);
-
- if (link_path)
- error = git_buf_put(link_path,
- path.ptr, path.size);
- }
-
- git_buf_free(&repo_link);
+ error = read_gitfile(&repo_link, path.ptr);
+ if (error < 0)
break;
+ if (valid_repository_path(&repo_link)) {
+ git_buf_swap(repo_path, &repo_link);
+
+ if (link_path)
+ error = git_buf_put(link_path, path.ptr, path.size);
}
- git_buf_free(&repo_link);
+ break;
}
}
- /* move up one directory level */
+ /* Move up one directory. If we're in_dot_git, we'll search the
+ * parent itself next. If we're !in_dot_git, we'll search .git
+ * in the parent directory next (added at the top of the loop). */
if (git_path_dirname_r(&path, path.ptr) < 0) {
error = -1;
break;
@@ -436,6 +436,12 @@ static int find_repo(
* find the ceiling for a search. */
if (min_iterations && (--min_iterations == 0))
ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs);
+
+ /* Check if we should stop searching here. */
+ if (min_iterations == 0
+ && (path.ptr[ceiling_offset] == 0
+ || (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))
+ break;
}
if (!error && parent_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
@@ -449,14 +455,16 @@ static int find_repo(
return -1;
}
- git_buf_free(&path);
-
+ /* If we didn't find the repository, and we don't have any other error
+ * to report, report that. */
if (!git_buf_len(repo_path) && !error) {
giterr_set(GITERR_REPOSITORY,
"Could not find repository from '%s'", start_path);
error = GIT_ENOTFOUND;
}
+ git_buf_free(&path);
+ git_buf_free(&repo_link);
return error;
}