Convert git_path_walk_up to regular function

This gets rid of the crazy macro version of git_path_walk_up
and makes it into a normal function that takes a callback
parameter.  This turned out not to be too messy.

src/attr.c

@@ -256,6 +256,17 @@ cleanup:
 #define push_attrs(R,S,B,F) \
 	git_attr_cache__push_file((R),(S),(B),(F),git_attr_file__from_file)
 
+typedef struct {
+	git_repository *repo;
+	git_vector *files;
+} attr_walk_up_info;
+
+static int push_one_attr(void *ref, git_buf *path)
+{
+	attr_walk_up_info *info = (attr_walk_up_info *)ref;
+	return push_attrs(info->repo, info->files, path->ptr, GIT_ATTR_FILE);
+}
+
 static int collect_attr_files(
 	git_repository *repo, const char *path, git_vector *files)
 {
@@ -263,6 +274,7 @@ static int collect_attr_files(
 	git_buf dir = GIT_BUF_INIT;
 	git_config *cfg;
 	const char *workdir = git_repository_workdir(repo);
+	attr_walk_up_info info;
 
 	if ((error = git_attr_cache__init(repo)) < GIT_SUCCESS)
 		goto cleanup;
@@ -284,20 +296,9 @@ static int collect_attr_files(
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 
-	if (workdir && git__prefixcmp(dir.ptr, workdir) == 0) {
+	info.repo = repo;
-		ssize_t rootlen = (ssize_t)strlen(workdir);
+	info.files = files;
-
+	error = git_path_walk_up(&dir, workdir, push_one_attr, &info);
-		do {
-			error = push_attrs(repo, files, dir.ptr, GIT_ATTR_FILE);
-			if (error == GIT_SUCCESS) {
-				git_path_dirname_r(&dir, dir.ptr);
-				git_path_to_dir(&dir);
-				error = git_buf_lasterror(&dir);
-			}
-		} while (!error && dir.size >= rootlen);
-	} else {
-		error = push_attrs(repo, files, dir.ptr, GIT_ATTR_FILE);
-	}
 	if (error < GIT_SUCCESS)
 		goto cleanup;
 

src/ignore.c

@@ -64,12 +64,24 @@ static int load_ignore_file(
 #define push_ignore(R,S,B,F) \
 	git_attr_cache__push_file((R),(S),(B),(F),load_ignore_file)
 
+typedef struct {
+	git_repository *repo;
+	git_vector *stack;
+} ignore_walk_up_info;
+
+static int push_one_ignore(void *ref, git_buf *path)
+{
+	ignore_walk_up_info *info = (ignore_walk_up_info *)ref;
+	return push_ignore(info->repo, info->stack, path->ptr, GIT_IGNORE_FILE);
+}
+
 int git_ignore__for_path(git_repository *repo, const char *path, git_vector *stack)
 {
 	int error = GIT_SUCCESS;
-	git_buf dir = GIT_BUF_INIT, scan;
+	git_buf dir = GIT_BUF_INIT;
 	git_config *cfg;
 	const char *workdir = git_repository_workdir(repo);
+	ignore_walk_up_info info;
 
 	if ((error = git_attr_cache__init(repo)) < GIT_SUCCESS)
 		goto cleanup;
@@ -82,11 +94,9 @@ int git_ignore__for_path(git_repository *repo, const char *path, git_vector *sta
 		goto cleanup;
 
 	/* load .gitignore up the path */
-	git_path_walk_up(&dir, &scan, workdir, {
+	info.repo = repo;
-		error = push_ignore(repo, stack, scan.ptr, GIT_IGNORE_FILE);
+	info.stack = stack;
-		if (error < GIT_SUCCESS) break;
+	if ((error = git_path_walk_up(&dir, workdir, push_one_ignore, &info)) < GIT_SUCCESS)
-	});
-	if (error < GIT_SUCCESS)
 		goto cleanup;
 
 	/* load .git/info/exclude */

src/path.c

@@ -305,3 +305,45 @@ int git_path_fromurl(git_buf *local_path_out, const char *file_url)
 
 	return error;
 }
+
+int git_path_walk_up(
+	git_buf *path,
+	const char *ceiling,
+	int (*cb)(void *data, git_buf *),
+	void *data)
+{
+	int error = GIT_SUCCESS;
+	git_buf iter;
+	ssize_t stop = 0, scan;
+	char oldc = '\0';
+
+	assert(path && cb);
+
+	if (ceiling != NULL) {
+		if (git__prefixcmp(path->ptr, ceiling) == GIT_SUCCESS)
+			stop = (ssize_t)strlen(ceiling);
+		else
+			stop = path->size;
+	}
+	scan = path->size;
+
+	iter.ptr = path->ptr;
+	iter.size = path->size;
+
+	while (scan >= stop) {
+		if ((error = cb(data, &iter)) < GIT_SUCCESS)
+			break;
+		iter.ptr[scan] = oldc;
+		scan = git_buf_rfind_next(&iter, '/');
+		if (scan >= 0) {
+			scan++;
+			oldc = iter.ptr[scan];
+			iter.size = scan;
+			iter.ptr[scan] = '\0';
+		}
+	}
+
+	iter.ptr[scan] = oldc;
+
+	return error;
+}

src/path.h

@@ -77,27 +77,18 @@ GIT_INLINE(void) git_path_mkposix(char *path)
 extern int git__percent_decode(git_buf *decoded_out, const char *input);
 extern int git_path_fromurl(git_buf *local_path_out, const char *file_url);
 
-/*
+/**
- * Use as:
+ * Invoke callback directory by directory up the path until the ceiling
+ * is reached (inclusive of a final call at the root_path).
  *
- *     git_path_walk_up(
+ * If the ceiling is NULL, this will walk all the way up to the root.
- *         git_buf *path, git_buf *iterator, const char *root_path,
+ * If the ceiling is not a prefix of the path, the callback will be
- *         ... CALLBACK CODE ...)
+ * invoked a single time on the verbatim input path.  Returning anything
- *
+ * other than GIT_SUCCESS from the callback function will stop the
- * to invoke callback directory by directory up the path until the root_path
+ * iteration and propogate the error to the caller.
- * is reached (inclusive of a final call at the root_path).  If root path is
- * NULL or the path is not contained in the root_path, then the callback
- * code will be invoked just once on input path.
  */
-#define git_path_walk_up(B,IB,ROOT,CODE) do { \
+extern int git_path_walk_up(
-	ssize_t _stop = ((ROOT) && git__prefixcmp((B)->ptr, (ROOT))) ? (ssize_t)strlen(ROOT) : (B)->size; \
+	git_buf *path, const char *ceiling,
-	ssize_t _scan = (B)->size; char _oldc = '\0'; \
+	int (*cb)(void *data, git_buf *), void *data);
-	(IB)->ptr = (B)->ptr; (IB)->size = (B)->size; \
-	while (_scan >= _stop) { \
-		CODE; \
-		(IB)->ptr[_scan] = _oldc; \
-		_scan = git_buf_rfind_next((IB), '/'); \
-		if (_scan >= 0) { _scan++; _oldc = (IB)->ptr[_scan]; (IB)->size = _scan; (IB)->ptr[_scan] = '\0'; } \
-	} (IB)->ptr[_scan] = _oldc; } while (0)
 
 #endif

tests-clay/core/path.c

@@ -337,9 +337,23 @@ void test_core_path__10_fromurl(void)
 	check_fromurl(ABS_PATH_MARKER "a", "file:///a", 0);
 }
 
+typedef struct {
+	int expect_idx;
+	char **expect;
+} check_walkup_info;
+
+static int check_one_walkup_step(void *ref, git_buf *path)
+{
+	check_walkup_info *info = (check_walkup_info *)ref;
+	cl_assert(info->expect[info->expect_idx] != NULL);
+	cl_assert_strequal(info->expect[info->expect_idx], path->ptr);
+	info->expect_idx++;
+	return GIT_SUCCESS;
+}
+
 void test_core_path__11_walkup(void)
 {
-	git_buf p = GIT_BUF_INIT, iter;
+	git_buf p = GIT_BUF_INIT;
 	char *expect[] = {
 		"/a/b/c/d/e/", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
 		"/a/b/c/d/e", "/a/b/c/d/", "/a/b/c/", "/a/b/", "/a/", "/", NULL,
@@ -353,16 +367,18 @@ void test_core_path__11_walkup(void)
 	};
 	char *root[] = { NULL, NULL, "/", "", "/a/b", "/a/b/", NULL, NULL, NULL };
 	int i, j;
+	check_walkup_info info;
+
+	info.expect = expect;
 
 	for (i = 0, j = 0; expect[i] != NULL; i++, j++) {
-		int cb_count = 0;
 
 		git_buf_sets(&p, expect[i]);
 
-		git_path_walk_up(&p, &iter, root[j], {
+		info.expect_idx = i;
-				cl_assert(expect[i + cb_count] != NULL);
+		cl_git_pass(
-				cl_assert_strequal(expect[i + cb_count], iter.ptr);
+			git_path_walk_up(&p, root[j], check_one_walkup_step, &info)
-				cb_count++; });
+		);
 
 		cl_assert_strequal(p.ptr, expect[i]);