Commit 2b490284 by Josh Triplett

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.
parent 0dd98b69
...@@ -357,6 +357,7 @@ static int find_repo( ...@@ -357,6 +357,7 @@ static int find_repo(
{ {
int error; int error;
git_buf path = GIT_BUF_INIT; git_buf path = GIT_BUF_INIT;
git_buf repo_link = GIT_BUF_INIT;
struct stat st; struct stat st;
dev_t initial_device = 0; dev_t initial_device = 0;
int min_iterations; int min_iterations;
...@@ -365,7 +366,8 @@ static int find_repo( ...@@ -365,7 +366,8 @@ static int find_repo(
git_buf_free(repo_path); 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; return error;
/* in_dot_git toggles each loop: /* in_dot_git toggles each loop:
...@@ -383,12 +385,13 @@ static int find_repo( ...@@ -383,12 +385,13 @@ static int find_repo(
min_iterations = 2; min_iterations = 2;
} }
while (!error && (min_iterations || !(path.ptr[ceiling_offset] == 0 || for (;;) {
(flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))) {
if (!(flags & GIT_REPOSITORY_OPEN_NO_DOTGIT)) { if (!(flags & GIT_REPOSITORY_OPEN_NO_DOTGIT)) {
if (!in_dot_git) if (!in_dot_git) {
if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0) error = git_buf_joinpath(&path, path.ptr, DOT_GIT);
if (error < 0)
break; break;
}
in_dot_git = !in_dot_git; in_dot_git = !in_dot_git;
} }
...@@ -397,7 +400,7 @@ static int find_repo( ...@@ -397,7 +400,7 @@ static int find_repo(
if (initial_device == 0) if (initial_device == 0)
initial_device = st.st_dev; initial_device = st.st_dev;
else if (st.st_dev != initial_device && else if (st.st_dev != initial_device &&
(flags & GIT_REPOSITORY_OPEN_CROSS_FS) == 0) !(flags & GIT_REPOSITORY_OPEN_CROSS_FS))
break; break;
if (S_ISDIR(st.st_mode)) { if (S_ISDIR(st.st_mode)) {
...@@ -408,25 +411,22 @@ static int find_repo( ...@@ -408,25 +411,22 @@ static int find_repo(
} }
} }
else if (S_ISREG(st.st_mode)) { else if (S_ISREG(st.st_mode)) {
git_buf repo_link = GIT_BUF_INIT; error = read_gitfile(&repo_link, path.ptr);
if (error < 0)
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);
break; 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) { if (git_path_dirname_r(&path, path.ptr) < 0) {
error = -1; error = -1;
break; break;
...@@ -436,6 +436,12 @@ static int find_repo( ...@@ -436,6 +436,12 @@ static int find_repo(
* find the ceiling for a search. */ * find the ceiling for a search. */
if (min_iterations && (--min_iterations == 0)) if (min_iterations && (--min_iterations == 0))
ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs); 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)) { if (!error && parent_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
...@@ -449,14 +455,16 @@ static int find_repo( ...@@ -449,14 +455,16 @@ static int find_repo(
return -1; 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) { if (!git_buf_len(repo_path) && !error) {
giterr_set(GITERR_REPOSITORY, giterr_set(GITERR_REPOSITORY,
"Could not find repository from '%s'", start_path); "Could not find repository from '%s'", start_path);
error = GIT_ENOTFOUND; error = GIT_ENOTFOUND;
} }
git_buf_free(&path);
git_buf_free(&repo_link);
return error; return error;
} }
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment