Initial iconv hookup for precomposed unicode

This hooks up git_path_direach and git_path_dirload so that they
will take a flag indicating if directory entry names should be
tested and converted from decomposed unicode to precomposed form.
This code will only come into play on the Apple platform and even
then, only when certain types of filesystems are used.

This involved adding a flag to these functions which involved
changing a lot of places in the code.

This was an opportunity to do a bit of code cleanup here and there,
for example, getting rid of the git_futils_cleanupdir_r function in
favor of a simple flag to git_futils_rmdir_r to not remove the top
level entry.  That ended up adding depth tracking during rmdir_r
which led to a safety check for infinite directory recursion.  Yay.

This hasn't actually been tested on the Mac filesystems where the
issue occurs.  I still need to get test environment for that.

CMakeLists.txt

@@ -58,7 +58,10 @@ FUNCTION(TARGET_OS_LIBRARIES target)
 		TARGET_LINK_LIBRARIES(${target} socket nsl)
 	ELSEIF(CMAKE_SYSTEM_NAME MATCHES "Linux")
 		TARGET_LINK_LIBRARIES(${target} rt)
-	ENDIF ()
+	ELSEIF(APPLE)
+		TARGET_LINK_LIBRARIES(${target} iconv)
+	ENDIF()
+
 	IF(THREADSAFE)
 		TARGET_LINK_LIBRARIES(${target} ${CMAKE_THREAD_LIBS_INIT})
 	ENDIF()

src/clone.c

@@ -391,11 +391,11 @@ int git_clone(
 	const char *local_path,
 	const git_clone_options *_options)
 {
-	int retcode = GIT_ERROR;
+	int error = 0;
 	git_repository *repo = NULL;
 	git_remote *origin;
 	git_clone_options options = GIT_CLONE_OPTIONS_INIT;
-	int remove_directory_on_failure = 0;
+	uint32_t rmdir_flags = GIT_RMDIR_REMOVE_FILES;
 
 	assert(out && url && local_path);
 
@@ -408,33 +408,29 @@ int git_clone(
 	if (git_path_exists(local_path) && !git_path_is_empty_dir(local_path)) {
 		giterr_set(GITERR_INVALID,
 			"'%s' exists and is not an empty directory", local_path);
-		return GIT_ERROR;
+		return GIT_EEXISTS;
 	}
 
-	/* Only remove the directory on failure if we create it */
-	remove_directory_on_failure = !git_path_exists(local_path);
+	/* Only remove the root directory on failure if we create it */
+	if (git_path_exists(local_path))
+		rmdir_flags |= GIT_RMDIR_SKIP_ROOT;
 
-	if ((retcode = git_repository_init(&repo, local_path, options.bare)) < 0)
-		return retcode;
+	if ((error = git_repository_init(&repo, local_path, options.bare)) < 0)
+		return error;
 
-	if ((retcode = create_and_configure_origin(&origin, repo, url, &options)) < 0)
-		goto cleanup;
+	if (!(error = create_and_configure_origin(&origin, repo, url, &options))) {
+		error = git_clone_into(
+			repo, origin, &options.checkout_opts, options.checkout_branch);
 
-	retcode = git_clone_into(repo, origin, &options.checkout_opts, options.checkout_branch);
-	git_remote_free(origin);
+		git_remote_free(origin);
+	}
 
-	if (retcode < 0)
-		goto cleanup;
+	if (error < 0) {
+		git_repository_free(repo);
+		repo = NULL;
+		(void)git_futils_rmdir_r(local_path, NULL, rmdir_flags);
+	}
 
 	*out = repo;
-	return 0;
-
-cleanup:
-	git_repository_free(repo);
-	if (remove_directory_on_failure)
-		git_futils_rmdir_r(local_path, NULL, GIT_RMDIR_REMOVE_FILES);
-	else
-		git_futils_cleanupdir_r(local_path);
-
-	return retcode;
+	return error;
 }

src/fileops.c

@@ -398,8 +398,11 @@ typedef struct {
 	size_t baselen;
 	uint32_t flags;
 	int error;
+	int depth;
 } futils__rmdir_data;
 
+#define FUTILS_MAX_DEPTH 100
+
 static int futils__error_cannot_rmdir(const char *path, const char *filemsg)
 {
 	if (filemsg)
@@ -441,7 +444,12 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 	struct stat st;
 	futils__rmdir_data *data = opaque;
 
-	if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) {
+	if (data->depth > FUTILS_MAX_DEPTH) {
+		data->error =
+			futils__error_cannot_rmdir(path->ptr, "directory nesting too deep");
+	}
+
+	else if ((data->error = p_lstat_posixly(path->ptr, &st)) < 0) {
 		if (errno == ENOENT)
 			data->error = 0;
 		else if (errno == ENOTDIR) {
@@ -457,9 +465,19 @@ static int futils__rmdir_recurs_foreach(void *opaque, git_buf *path)
 	}
 
 	else if (S_ISDIR(st.st_mode)) {
-		int error = git_path_direach(path, futils__rmdir_recurs_foreach, data);
-		if (error < 0)
-			return (error == GIT_EUSER) ? data->error : error;
+		data->depth++;
+
+		{
+			int error =
+				git_path_direach(path, 0, futils__rmdir_recurs_foreach, data);
+			if (error < 0)
+				return (error == GIT_EUSER) ? data->error : error;
+		}
+
+		data->depth--;
+
+		if (data->depth == 0 && (data->flags & GIT_RMDIR_SKIP_ROOT) != 0)
+			return data->error;
 
 		data->error = p_rmdir(path->ptr);
 
@@ -517,7 +535,7 @@ int git_futils_rmdir_r(
 {
 	int error;
 	git_buf fullpath = GIT_BUF_INIT;
-	futils__rmdir_data data;
+	futils__rmdir_data data = { 0 };
 
 	/* build path and find "root" where we should start calling mkdir */
 	if (git_path_join_unrooted(&fullpath, path, base, NULL) < 0)
@@ -526,7 +544,6 @@ int git_futils_rmdir_r(
 	data.base    = base ? base : "";
 	data.baselen = base ? strlen(base) : 0;
 	data.flags   = flags;
-	data.error   = 0;
 
 	error = futils__rmdir_recurs_foreach(&data, &fullpath);
 
@@ -544,41 +561,6 @@ int git_futils_rmdir_r(
 	return error;
 }
 
-int git_futils_cleanupdir_r(const char *path)
-{
-	int error;
-	git_buf fullpath = GIT_BUF_INIT;
-	futils__rmdir_data data;
-
-	if ((error = git_buf_put(&fullpath, path, strlen(path))) < 0)
-		goto clean_up;
-
-	data.base    = "";
-	data.baselen = 0;
-	data.flags   = GIT_RMDIR_REMOVE_FILES;
-	data.error   = 0;
-
-	if (!git_path_exists(path)) {
-		giterr_set(GITERR_OS, "Path does not exist: %s" , path);
-		error = GIT_ERROR;
-		goto clean_up;
-	}
-
-	if (!git_path_isdir(path)) {
-		giterr_set(GITERR_OS, "Path is not a directory: %s" , path);
-		error = GIT_ERROR;
-		goto clean_up;
-	}
-
-	error = git_path_direach(&fullpath, futils__rmdir_recurs_foreach, &data);
-	if (error == GIT_EUSER)
-		error = data.error;
-
-clean_up:
-	git_buf_free(&fullpath);
-	return error;
-}
-
 
 static int git_futils_guess_system_dirs(git_buf *out)
 {
@@ -948,9 +930,12 @@ static int _cp_r_callback(void *ref, git_buf *from)
 			error = _cp_r_mkdir(info, from);
 
 		/* recurse onto target directory */
-		if (!error && (!exists || S_ISDIR(to_st.st_mode)) &&
-			((error = git_path_direach(from, _cp_r_callback, info)) == GIT_EUSER))
-			error = info->error;
+		if (!error && (!exists || S_ISDIR(to_st.st_mode))) {
+			error = git_path_direach(from, 0, _cp_r_callback, info);
+
+			if (error == GIT_EUSER)
+				error = info->error;
+		}
 
 		if (oldmode != 0)
 			info->dirmode = oldmode;

src/fileops.h

@@ -115,12 +115,7 @@ extern int git_futils_mkpath2file(const char *path, const mode_t mode);
  * * GIT_RMDIR_EMPTY_PARENTS   - remove containing directories up to base
  *       if removing this item leaves them empty
  * * GIT_RMDIR_REMOVE_BLOCKERS - remove blocking file that causes ENOTDIR
- *
- * The old values translate into the new as follows:
- *
- * * GIT_DIRREMOVAL_EMPTY_HIERARCHY == GIT_RMDIR_EMPTY_HIERARCHY
- * * GIT_DIRREMOVAL_FILES_AND_DIRS  ~= GIT_RMDIR_REMOVE_FILES
- * * GIT_DIRREMOVAL_ONLY_EMPTY_DIRS == GIT_RMDIR_SKIP_NONEMPTY
+ * * GIT_RMDIR_SKIP_ROOT       - don't remove root directory itself
  */
 typedef enum {
 	GIT_RMDIR_EMPTY_HIERARCHY = 0,
@@ -128,6 +123,7 @@ typedef enum {
 	GIT_RMDIR_SKIP_NONEMPTY   = (1 << 1),
 	GIT_RMDIR_EMPTY_PARENTS   = (1 << 2),
 	GIT_RMDIR_REMOVE_BLOCKERS = (1 << 3),
+	GIT_RMDIR_SKIP_ROOT       = (1 << 4),
 } git_futils_rmdir_flags;
 
 /**
@@ -141,14 +137,6 @@ typedef enum {
 extern int git_futils_rmdir_r(const char *path, const char *base, uint32_t flags);
 
 /**
- * Remove all files and directories beneath the specified path.
- *
- * @param path Path to the top level directory to process.
- * @return 0 on success; -1 on error.
- */
-extern int git_futils_cleanupdir_r(const char *path);
-
-/**
  * Create and open a temporary file with a `_git2_` suffix.
  * Writes the filename into path_out.
  * @return On success, an open file descriptor, else an error code < 0.

src/iterator.c

@@ -893,6 +893,7 @@ struct fs_iterator {
 	git_index_entry entry;
 	git_buf path;
 	size_t root_len;
+	uint32_t dirload_flags;
 	int depth;
 
 	int (*enter_dir_cb)(fs_iterator *self);
@@ -986,10 +987,7 @@ static int fs_iterator__expand_dir(fs_iterator *fi)
 	GITERR_CHECK_ALLOC(ff);
 
 	error = git_path_dirload_with_stat(
-		fi->path.ptr, fi->root_len,
-		(iterator__ignore_case(fi) ? GIT_PATH_DIRLOAD_IGNORE_CASE : 0) |
-		(iterator__flag(fi, PRECOMPOSE_UNICODE) ?
-			GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE : 0),
+		fi->path.ptr, fi->root_len, fi->dirload_flags,
 		fi->base.start, fi->base.end, &ff->entries);
 
 	if (error < 0) {
@@ -1210,6 +1208,11 @@ static int fs_iterator__initialize(
 	}
 	fi->root_len = fi->path.size;
 
+	fi->dirload_flags =
+		(iterator__ignore_case(fi) ? GIT_PATH_DIR_IGNORE_CASE : 0) |
+		(iterator__flag(fi, PRECOMPOSE_UNICODE) ?
+			GIT_PATH_DIR_PRECOMPOSE_UNICODE : 0);
+
 	if ((error = fs_iterator__expand_dir(fi)) < 0) {
 		if (error == GIT_ENOTFOUND || error == GIT_ITEROVER) {
 			giterr_clear();
@@ -1332,7 +1335,7 @@ int git_iterator_for_workdir_ext(
 	const char *start,
 	const char *end)
 {
-	int error;
+	int error, precompose = 0;
 	workdir_iterator *wi;
 
 	if (!repo_workdir) {
@@ -1360,13 +1363,10 @@ int git_iterator_for_workdir_ext(
 	}
 
 	/* try to look up precompose and set flag if appropriate */
-	{
-		int precompose = 0;
-		if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0)
-			giterr_clear();
-		else if (precompose)
-			wi->fi.base.flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE;
-	}
+	if (git_repository__cvar(&precompose, repo, GIT_CVAR_PRECOMPOSE) < 0)
+		giterr_clear();
+	else if (precompose)
+		wi->fi.base.flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE;
 
 	return fs_iterator__initialize(out, &wi->fi, repo_workdir);
 }

src/odb_loose.c

@@ -544,7 +544,7 @@ static int locate_object_short_oid(
 
 	/* Explore directory to find a unique object matching short_oid */
 	error = git_path_direach(
-		object_location, fn_locate_object_short_oid, &state);
+		object_location, 0, fn_locate_object_short_oid, &state);
 
 	if (error && error != GIT_EUSER)
 		return error;
@@ -745,7 +745,7 @@ static int foreach_cb(void *_state, git_buf *path)
 {
 	struct foreach_state *state = (struct foreach_state *) _state;
 
-	return git_path_direach(path, foreach_object_dir_cb, state);
+	return git_path_direach(path, 0, foreach_object_dir_cb, state);
 }
 
 static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb cb, void *data)
@@ -768,7 +768,7 @@ static int loose_backend__foreach(git_odb_backend *_backend, git_odb_foreach_cb
 	state.data = data;
 	state.dir_len = git_buf_len(&buf);
 
-	error = git_path_direach(&buf, foreach_cb, &state);
+	error = git_path_direach(&buf, 0, foreach_cb, &state);
 
 	git_buf_free(&buf);
 

src/odb_pack.c

@@ -331,7 +331,7 @@ static int pack_backend__refresh(git_odb_backend *_backend)
 	git_buf_sets(&path, backend->pack_folder);
 
 	/* reload all packs */
-	error = git_path_direach(&path, packfile_load__cb, (void *)backend);
+	error = git_path_direach(&path, 0, packfile_load__cb, backend);
 
 	git_buf_free(&path);
 

src/path.c

@@ -18,6 +18,12 @@
 
 #define LOOKS_LIKE_DRIVE_PREFIX(S) (git__isalpha((S)[0]) && (S)[1] == ':')
 
+#if __APPLE__
+#include <iconv.h>
+#endif
+#define ICONV_REPO_ENCODING "UTF-8"
+#define ICONV_PATH_ENCODING "UTF-8-MAC"
+
 #ifdef GIT_WIN32
 static bool looks_like_network_computer_name(const char *path, int pos)
 {
@@ -724,14 +730,83 @@ int git_path_cmp(
 	return (c1 < c2) ? -1 : (c1 > c2) ? 1 : 0;
 }
 
+static bool path_has_non_ascii(const char *path, size_t pathlen)
+{
+	const uint8_t *scan = (const uint8_t *)path, *end;
+
+	for (end = scan + pathlen; scan < end; ++scan)
+		if (*scan & 0x80)
+			return true;
+
+	return false;
+}
+
+#ifdef __APPLE__
+static int path_iconv(iconv_t map, git_buf *out, char **in, size_t *inlen)
+{
+	char *nfd = *in, *nfc;
+	size_t nfdlen = *inlen, nfclen, wantlen = nfdlen, rv;
+	int retry = 1;
+
+	if (!path_has_non_ascii(*in, *inlen))
+		return 0;
+
+	while (1) {
+		if (git_buf_grow(out, wantlen) < 0)
+			return -1;
+
+		nfc    = out->ptr   + out->size;
+		nfclen = out->asize - out->size;
+
+		rv = iconv(map, &nfd, &nfdlen, &nfc, &nfclen);
+
+		out->size = (nfc - out->ptr);
+
+		if (rv != (size_t)-1)
+			break;
+
+		if (errno != E2BIG)
+			return -1;
+
+		/* make space for 2x the remaining data to be converted
+		 * (with per retry overhead to avoid infinite loops)
+		 */
+		wantlen = out->size + max(nfclen, nfdlen) * 2 + (size_t)(retry * 4);
+
+		if (retry++ > 4)
+			return -1;
+	}
+
+	out->ptr[out->size] = '\0';
+
+	*in    = out->ptr;
+	*inlen = out->size;
+
+	return 0;
+}
+#endif
+
+#if defined(__sun) || defined(__GNU__)
+typedef char path_dirent_data[sizeof(struct dirent) + FILENAME_MAX + 1];
+#else
+typedef struct dirent path_dirent_data;
+#endif
+
 int git_path_direach(
 	git_buf *path,
+	uint32_t flags,
 	int (*fn)(void *, git_buf *),
 	void *arg)
 {
+	int error = 0;
 	ssize_t wd_len;
 	DIR *dir;
-	struct dirent *de, *de_buf;
+	path_dirent_data de_data;
+	struct dirent *de, *de_buf = (struct dirent *)&de_data;
+#ifdef __APPLE__
+	iconv_t nfd2nfc = (iconv_t)-1;
+	git_buf nfc_path = GIT_BUF_INIT;
+#endif
 
 	if (git_path_to_dir(path) < 0)
 		return -1;
@@ -743,38 +818,46 @@ int git_path_direach(
 		return -1;
 	}
 
-#if defined(__sun) || defined(__GNU__)
-	de_buf = git__malloc(sizeof(struct dirent) + FILENAME_MAX + 1);
-#else
-	de_buf = git__malloc(sizeof(struct dirent));
+#ifdef __APPLE__
+	if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0)
+		nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING);
 #endif
 
 	while (p_readdir_r(dir, de_buf, &de) == 0 && de != NULL) {
-		int result;
+		char *de_path = de->d_name;
+		size_t de_len = strlen(de_path);
 
-		if (git_path_is_dot_or_dotdot(de->d_name))
+		if (git_path_is_dot_or_dotdot(de_path))
 			continue;
 
-		if (git_buf_puts(path, de->d_name) < 0) {
-			closedir(dir);
-			git__free(de_buf);
-			return -1;
-		}
+#if __APPLE__
+		if (nfd2nfc != (iconv_t)-1 &&
+			(error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0)
+			break;
+#endif
 
-		result = fn(arg, path);
+		if ((error = git_buf_put(path, de_path, de_len)) < 0)
+			break;
+
+		error = fn(arg, path);
 
 		git_buf_truncate(path, wd_len); /* restore path */
 
-		if (result) {
-			closedir(dir);
-			git__free(de_buf);
-			return GIT_EUSER;
+		if (error) {
+			error = GIT_EUSER;
+			break;
 		}
 	}
 
 	closedir(dir);
-	git__free(de_buf);
-	return 0;
+
+#ifdef __APPLE__
+	if (nfd2nfc != (iconv_t)-1)
+		iconv_close(nfd2nfc);
+	git_buf_free(&nfc_path);
+#endif
+
+	return error;
 }
 
 int git_path_dirload(
@@ -786,22 +869,30 @@ int git_path_dirload(
 {
 	int error, need_slash;
 	DIR *dir;
-	struct dirent *de, *de_buf;
 	size_t path_len;
+	path_dirent_data de_data;
+	struct dirent *de, *de_buf = (struct dirent *)&de_data;
+#ifdef __APPLE__
+	iconv_t nfd2nfc = (iconv_t)-1;
+	git_buf nfc_path = GIT_BUF_INIT;
+#endif
+
+	assert(path && contents);
 
-	assert(path != NULL && contents != NULL);
 	path_len = strlen(path);
-	assert(path_len > 0 && path_len >= prefix_len);
 
+	if (!path_len || path_len < prefix_len) {
+		giterr_set(GITERR_INVALID, "Invalid directory path '%s'", path);
+		return -1;
+	}
 	if ((dir = opendir(path)) == NULL) {
 		giterr_set(GITERR_OS, "Failed to open directory '%s'", path);
 		return -1;
 	}
 
-#if defined(__sun) || defined(__GNU__)
-	de_buf = git__malloc(sizeof(struct dirent) + FILENAME_MAX + 1);
-#else
-	de_buf = git__malloc(sizeof(struct dirent));
+#ifdef __APPLE__
+	if ((flags & GIT_PATH_DIR_PRECOMPOSE_UNICODE) != 0)
+		nfd2nfc = iconv_open(ICONV_REPO_ENCODING, ICONV_PATH_ENCODING);
 #endif
 
 	path += prefix_len;
@@ -809,40 +900,41 @@ int git_path_dirload(
 	need_slash = (path_len > 0 && path[path_len-1] != '/') ? 1 : 0;
 
 	while ((error = p_readdir_r(dir, de_buf, &de)) == 0 && de != NULL) {
-		char *entry_path;
-		size_t entry_len;
+		char *entry_path, *de_path = de->d_name;
+		size_t alloc_size, de_len = strlen(de_path);
 
-		if (git_path_is_dot_or_dotdot(de->d_name))
+		if (git_path_is_dot_or_dotdot(de_path))
 			continue;
 
-		entry_len = strlen(de->d_name);
+#if __APPLE__
+		if (nfd2nfc != (iconv_t)-1 &&
+			(error = path_iconv(nfd2nfc, &nfc_path, &de_path, &de_len)) < 0)
+			break;
+#endif
 
-		/* if we read decomposed unicode and precompose flag is set,
-		 * then precompose it now so app code sees it as precomposed
-		 */
-		if ((flags & GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE) != 0) {
+		alloc_size = path_len + need_slash + de_len + 1 + alloc_extra;
+		if ((entry_path = git__calloc(alloc_size, 1)) == NULL) {
+			error = -1;
+			break;
 		}
 
-		entry_path = git__malloc(
-			path_len + need_slash + entry_len + 1 + alloc_extra);
-		GITERR_CHECK_ALLOC(entry_path);
-
 		if (path_len)
 			memcpy(entry_path, path, path_len);
 		if (need_slash)
 			entry_path[path_len] = '/';
-		memcpy(&entry_path[path_len + need_slash], de->d_name, entry_len);
-		entry_path[path_len + need_slash + entry_len] = '\0';
+		memcpy(&entry_path[path_len + need_slash], de_path, de_len);
 
-		if (git_vector_insert(contents, entry_path) < 0) {
-			closedir(dir);
-			git__free(de_buf);
-			return -1;
-		}
+		if ((error = git_vector_insert(contents, entry_path)) < 0)
+			break;
 	}
 
 	closedir(dir);
-	git__free(de_buf);
+
+#ifdef __APPLE__
+	if (nfd2nfc != (iconv_t)-1)
+		iconv_close(nfd2nfc);
+	git_buf_free(&nfc_path);
+#endif
 
 	if (error != 0)
 		giterr_set(GITERR_OS, "Failed to process directory entry in '%s'", path);
@@ -888,7 +980,7 @@ int git_path_dirload_with_stat(
 		return error;
 	}
 
-	strncomp = (flags & GIT_PATH_DIRLOAD_IGNORE_CASE) != 0 ?
+	strncomp = (flags & GIT_PATH_DIR_IGNORE_CASE) != 0 ?
 		git__strncasecmp : git__strncmp;
 
 	/* stat struct at start of git_path_with_stat, so shift path text */

src/path.h

@@ -242,21 +242,28 @@ extern int git_path_resolve_relative(git_buf *path, size_t ceiling);
  */
 extern int git_path_apply_relative(git_buf *target, const char *relpath);
 
+enum {
+	GIT_PATH_DIR_IGNORE_CASE = (1u << 0),
+	GIT_PATH_DIR_PRECOMPOSE_UNICODE = (1u << 1),
+};
+
 /**
  * Walk each directory entry, except '.' and '..', calling fn(state).
  *
- * @param pathbuf buffer the function reads the initial directory
+ * @param pathbuf Buffer the function reads the initial directory
  * 		path from, and updates with each successive entry's name.
- * @param fn function to invoke with each entry. The first arg is
- *		the input state and the second arg is pathbuf. The function
- *		may modify the pathbuf, but only by appending new text.
- * @param state to pass to fn as the first arg.
+ * @param flags Combination of GIT_PATH_DIR flags.
+ * @param callback Callback for each entry. Passed the `payload` and each
+ *		successive path inside the directory as a full path.  This may
+ *		safely append text to the pathbuf if needed.
+ * @param payload Passed to callback as first argument.
  * @return 0 on success, GIT_EUSER on non-zero callback, or error code
  */
 extern int git_path_direach(
 	git_buf *pathbuf,
-	int (*fn)(void *, git_buf *),
-	void *state);
+	uint32_t flags,
+	int (*callback)(void *payload, git_buf *path),
+	void *payload);
 
 /**
  * Sort function to order two paths
@@ -276,24 +283,19 @@ extern int git_path_cmp(
  * @param pathbuf Buffer the function reads the directory from and
  *		and updates with each successive name.
  * @param ceiling Prefix of path at which to stop walking up.  If NULL,
- *      this will walk all the way up to the root.  If not a prefix of
- *      pathbuf, the callback will be invoked a single time on the
- *      original input path.
- * @param fn Function to invoke on each path.  The first arg is the
- *		input satte and the second arg is the pathbuf.  The function
- *		should not modify the pathbuf.
+ *		this will walk all the way up to the root.  If not a prefix of
+ *		pathbuf, the callback will be invoked a single time on the
+ *		original input path.
+ * @param callback Function to invoke on each path.  Passed the `payload`
+ *		and the buffer containing the current path.  The path should not
+ *		be modified in any way.
  * @param state Passed to fn as the first ath.
  */
 extern int git_path_walk_up(
 	git_buf *pathbuf,
 	const char *ceiling,
-	int (*fn)(void *state, git_buf *),
-	void *state);
-
-enum {
-	GIT_PATH_DIRLOAD_IGNORE_CASE = (1u << 0),
-	GIT_PATH_DIRLOAD_PRECOMPOSE_UNICODE = (1u << 1),
-};
+	int (*callback)(void *payload, git_buf *path),
+	void *payload);
 
 /**
  * Load all directory entries (except '.' and '..') into a vector.
@@ -309,13 +311,14 @@ enum {
  * 		prefix_len 3, the entries will look like "b/e1", "b/e2", etc.
  * @param alloc_extra Extra bytes to add to each string allocation in
  * 		case you want to append anything funny.
+ * @param flags Combination of GIT_PATH_DIR flags.
  * @param contents Vector to fill with directory entry names.
  */
 extern int git_path_dirload(
 	const char *path,
 	size_t prefix_len,
 	size_t alloc_extra,
-	unsigned int flags,
+	uint32_t flags,
 	git_vector *contents);
 
 
@@ -342,7 +345,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b);
  *
  * @param path The directory to read from
  * @param prefix_len The trailing part of path to prefix to entry paths
- * @param flags GIT_PATH_DIRLOAD flags from above
+ * @param flags GIT_PATH_DIR flags from above
  * @param start_stat As optimization, only stat values after this prefix
  * @param end_stat As optimization, only stat values before this prefix
  * @param contents Vector to fill with git_path_with_stat structures
@@ -350,7 +353,7 @@ extern int git_path_with_stat_cmp_icase(const void *a, const void *b);
 extern int git_path_dirload_with_stat(
 	const char *path,
 	size_t prefix_len,
-	unsigned int flags,
+	uint32_t flags,
 	const char *start_stat,
 	const char *end_stat,
 	git_vector *contents);

src/refdb_fs.c

@@ -56,6 +56,8 @@ typedef struct refdb_fs_backend {
 
 	git_sortedcache *refcache;
 	int peeling_mode;
+	git_iterator_flag_t iterator_flags;
+	uint32_t direach_flags;
 } refdb_fs_backend;
 
 static int packref_cmp(const void *a_, const void *b_)
@@ -269,7 +271,8 @@ static int _dirent_loose_load(void *data, git_buf *full_path)
 		return 0;
 
 	if (git_path_isdir(full_path->ptr))
-		return git_path_direach(full_path, _dirent_loose_load, backend);
+		return git_path_direach(
+			full_path, backend->direach_flags, _dirent_loose_load, backend);
 
 	file_path = full_path->ptr + strlen(backend->path);
 
@@ -295,7 +298,8 @@ static int packed_loadloose(refdb_fs_backend *backend)
 	 * This will overwrite any old packed entries with their
 	 * updated loose versions
 	 */
-	error = git_path_direach(&refs_path, _dirent_loose_load, backend);
+	error = git_path_direach(
+		&refs_path, backend->direach_flags, _dirent_loose_load, backend);
 
 	git_buf_free(&refs_path);
 
@@ -458,23 +462,17 @@ static void refdb_fs_backend__iterator_free(git_reference_iterator *_iter)
 
 static int iter_load_loose_paths(refdb_fs_backend *backend, refdb_fs_iter *iter)
 {
-	int error = 0, t;
+	int error = 0;
 	git_buf path = GIT_BUF_INIT;
-	git_iterator_flag_t flags = 0;
 	git_iterator *fsit = NULL;
 	const git_index_entry *entry = NULL;
 
 	if (!backend->path) /* do nothing if no path for loose refs */
 		return 0;
 
-	if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_IGNORECASE) && t)
-		flags |= GIT_ITERATOR_IGNORE_CASE;
-	if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_PRECOMPOSE) && t)
-		flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE;
-
 	if ((error = git_buf_printf(&path, "%s/refs", backend->path)) < 0 ||
 		(error = git_iterator_for_filesystem(
-			&fsit, git_buf_cstr(&path), flags, NULL, NULL)) < 0) {
+			&fsit, path.ptr, backend->iterator_flags, NULL, NULL)) < 0) {
 		git_buf_free(&path);
 		return error;
 	}
@@ -1077,6 +1075,7 @@ int git_refdb_backend_fs(
 	git_refdb_backend **backend_out,
 	git_repository *repository)
 {
+	int t = 0;
 	git_buf path = GIT_BUF_INIT;
 	refdb_fs_backend *backend;
 
@@ -1098,6 +1097,15 @@ int git_refdb_backend_fs(
 
 	git_buf_free(&path);
 
+	if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_IGNORECASE) && t) {
+		backend->iterator_flags |= GIT_ITERATOR_IGNORE_CASE;
+		backend->direach_flags  |= GIT_PATH_DIR_IGNORE_CASE;
+	}
+	if (!git_repository__cvar(&t, backend->repo, GIT_CVAR_PRECOMPOSE) && t) {
+		backend->iterator_flags |= GIT_ITERATOR_PRECOMPOSE_UNICODE;
+		backend->direach_flags  |= GIT_PATH_DIR_PRECOMPOSE_UNICODE;
+	}
+
 	backend->parent.exists = &refdb_fs_backend__exists;
 	backend->parent.lookup = &refdb_fs_backend__lookup;
 	backend->parent.iterator = &refdb_fs_backend__iterator;

tests-clar/clar_libgit2.c

@@ -301,7 +301,7 @@ static int remove_placeholders_recurs(void *_data, git_buf *path)
 	size_t pathlen;
 
 	if (git_path_isdir(path->ptr) == true)
-		return git_path_direach(path, remove_placeholders_recurs, data);
+		return git_path_direach(path, 0, remove_placeholders_recurs, data);
 
 	pathlen = path->size;
 

tests-clar/clone/nonetwork.c

@@ -63,34 +63,27 @@ static int dont_call_me(void *state, git_buf *path)
 	return GIT_ERROR;
 }
 
-void test_clone_nonetwork__do_not_clean_existing_directory(void)
+static void assert_empty_directory(const char *path)
 {
-	git_buf path_buf = GIT_BUF_INIT;
-
-	git_buf_put(&path_buf, "./foo", 5);
+	git_buf buf = GIT_BUF_INIT;
+	cl_assert(git_path_exists(path));
+	cl_git_pass(git_buf_sets(&buf, path));
+	cl_git_pass(git_path_direach(&buf, 0, dont_call_me, NULL));
+	git_buf_free(&buf);
+}
 
+void test_clone_nonetwork__do_not_clean_existing_directory(void)
+{
 	/* Clone should not remove the directory if it already exists, but
 	 * Should clean up entries it creates. */
 	p_mkdir("./foo", GIT_DIR_MODE);
 	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
-	cl_assert(git_path_exists("./foo"));
-
-	/* Make sure the directory is empty. */
-	cl_git_pass(git_path_direach(&path_buf,
-		dont_call_me,
-		NULL));
+	assert_empty_directory("./foo");
 
 	/* Try again with a bare repository. */
 	g_options.bare = true;
 	cl_git_fail(git_clone(&g_repo, "not_a_repo", "./foo", &g_options));
-	cl_assert(git_path_exists("./foo"));
-
-	/* Make sure the directory is empty. */
-	cl_git_pass(git_path_direach(&path_buf,
-		dont_call_me,
-		NULL));
-
-	git_buf_free(&path_buf);
+	assert_empty_directory("./foo");
 }
 
 void test_clone_nonetwork__local(void)

tests-clar/core/dirent.c

@@ -115,9 +115,7 @@ void test_core_dirent__dont_traverse_dot(void)
 	cl_set_cleanup(&dirent_cleanup__cb, &dot);
 	setup(&dot);
 
-	cl_git_pass(git_path_direach(&dot.path,
-					one_entry,
-					&dot));
+	cl_git_pass(git_path_direach(&dot.path, 0, one_entry, &dot));
 
 	check_counts(&dot);
 }
@@ -141,9 +139,7 @@ void test_core_dirent__traverse_subfolder(void)
 	cl_set_cleanup(&dirent_cleanup__cb, &sub);
 	setup(&sub);
 
-	cl_git_pass(git_path_direach(&sub.path,
-					one_entry,
-					&sub));
+	cl_git_pass(git_path_direach(&sub.path, 0, one_entry, &sub));
 
 	check_counts(&sub);
 }
@@ -161,9 +157,7 @@ void test_core_dirent__traverse_slash_terminated_folder(void)
 	cl_set_cleanup(&dirent_cleanup__cb, &sub_slash);
 	setup(&sub_slash);
 
-	cl_git_pass(git_path_direach(&sub_slash.path,
-					one_entry,
-					&sub_slash));
+	cl_git_pass(git_path_direach(&sub_slash.path, 0, one_entry, &sub_slash));
 
 	check_counts(&sub_slash);
 }
@@ -184,16 +178,12 @@ void test_core_dirent__dont_traverse_empty_folders(void)
 	cl_set_cleanup(&dirent_cleanup__cb, &empty);
 	setup(&empty);
 
-	cl_git_pass(git_path_direach(&empty.path,
-					one_entry,
-					&empty));
+	cl_git_pass(git_path_direach(&empty.path, 0, one_entry, &empty));
 
 	check_counts(&empty);
 
 	/* make sure callback not called */
-	cl_git_pass(git_path_direach(&empty.path,
-					dont_call_me,
-					&empty));
+	cl_git_pass(git_path_direach(&empty.path, 0, dont_call_me, &empty));
 }
 
 static name_data odd_names[] = {
@@ -216,9 +206,7 @@ void test_core_dirent__traverse_weird_filenames(void)
 	cl_set_cleanup(&dirent_cleanup__cb, &odd);
 	setup(&odd);
 
-	cl_git_pass(git_path_direach(&odd.path,
-					one_entry,
-					&odd));
+	cl_git_pass(git_path_direach(&odd.path, 0, one_entry, &odd));
 
 	check_counts(&odd);
 }

tests-clar/merge/merge_helpers.c

@@ -291,7 +291,7 @@ int merge_test_workdir(git_repository *repo, const struct merge_index_entry expe
 	git_buf wd = GIT_BUF_INIT;
 
 	git_buf_puts(&wd, repo->workdir);
-	git_path_direach(&wd, dircount, &actual_len);
+	git_path_direach(&wd, 0, dircount, &actual_len);
 
 	if (actual_len != expected_len)
 		return 0;

tests-clar/status/worktree.c

@@ -119,7 +119,7 @@ void test_status_worktree__purged_worktree(void)
 
 	/* first purge the contents of the worktree */
 	cl_git_pass(git_buf_sets(&workdir, git_repository_workdir(repo)));
-	cl_git_pass(git_path_direach(&workdir, remove_file_cb, NULL));
+	cl_git_pass(git_path_direach(&workdir, 0, remove_file_cb, NULL));
 	git_buf_free(&workdir);
 
 	/* now get status */