Commit 5c387b6c5a616d245e51e4ca1935e6ffd78c710e

Edward Thomson 2015-04-29T14:31:59

git_path_diriter: next shouldn't take path ptr The _next method shouldn't take a path pointer (and a path_len pointer) as 100% of current users use the full path and ignore the filename. Plus let's add some docs and a unit test.

diff --git a/src/iterator.c b/src/iterator.c
index 7e89b77..52814ba 100644
--- a/src/iterator.c
+++ b/src/iterator.c
@@ -1026,7 +1026,7 @@ static int dirload_with_stat(
 	if ((error = git_path_diriter_init(&diriter, dirpath, flags)) < 0)
 		goto done;
 
-	while ((error = git_path_diriter_next(&path, &path_len, &diriter)) == 0) {
+	while ((error = git_path_diriter_next(&diriter)) == 0) {
 		if ((error = git_path_diriter_fullpath(&path, &path_len, &diriter)) < 0)
 			goto done;
 
diff --git a/src/path.c b/src/path.c
index d8f3c23..ee56698 100644
--- a/src/path.c
+++ b/src/path.c
@@ -1111,10 +1111,7 @@ int git_path_diriter_init(
 	return 0;
 }
 
-int git_path_diriter_next(
-	const char **out,
-	size_t *out_len,
-	git_path_diriter *diriter)
+int git_path_diriter_next(git_path_diriter *diriter)
 {
 	struct dirent *de;
 	const char *filename;
@@ -1122,10 +1119,7 @@ int git_path_diriter_next(
 	bool skip_dot = !(diriter->flags & GIT_PATH_DIR_INCLUDE_DOT_AND_DOTDOT);
 	int error = 0;
 
-	assert(out && out_len && diriter);
-
-	*out = NULL;
-	*out_len = 0;
+	assert(diriter);
 
 	errno = 0;
 
@@ -1155,12 +1149,21 @@ int git_path_diriter_next(
 	if (git_buf_oom(&diriter->path))
 		return -1;
 
-	*out = &diriter->path.ptr[diriter->parent_len+1];
-	*out_len = filename_len;
-
 	return error;
 }
 
+int git_path_diriter_filename(
+	const char **out,
+	size_t *out_len,
+	git_path_diriter *diriter)
+{
+	assert(out && out_len && diriter);
+
+	*out = &diriter->path.ptr[diriter->parent_len+1];
+	*out_len = diriter->path.size - diriter->parent_len - 1;
+	return 0;
+}
+
 int git_path_diriter_fullpath(
 	const char **out,
 	size_t *out_len,
@@ -1214,7 +1217,7 @@ int git_path_dirload(
 	if ((error = git_path_diriter_init(&iter, path, flags)) < 0)
 		return error;
 
-	while ((error = git_path_diriter_next(&name, &name_len, &iter)) == 0) {
+	while ((error = git_path_diriter_next(&iter)) == 0) {
 		if ((error = git_path_diriter_fullpath(&name, &name_len, &iter)) < 0)
 			break;
 
diff --git a/src/path.h b/src/path.h
index 4900dce..927d2fc 100644
--- a/src/path.h
+++ b/src/path.h
@@ -339,23 +339,70 @@ struct git_path_diriter
 	DIR *dir;
 };
 
+/**
+ * Initialize a directory iterator.
+ *
+ * @param diriter Pointer to a diriter structure that will be setup.
+ * @param path The path that will be iterated over
+ * @param flags Directory reader flags
+ * @return 0 or an error code
+ */
 extern int git_path_diriter_init(
 	git_path_diriter *diriter,
 	const char *path,
 	unsigned int flags);
 
-extern int git_path_diriter_next(
+/**
+ * Advance the directory iterator.  Will return GIT_ITEROVER when
+ * the iteration has completed successfully.
+ *
+ * @param diriter The directory iterator
+ * @return 0, GIT_ITEROVER, or an error code
+ */
+extern int git_path_diriter_next(git_path_diriter *diriter);
+
+/**
+ * Returns the file name of the current item in the iterator.
+ *
+ * @param out Pointer to store the path in
+ * @param out_len Pointer to store the length of the path in
+ * @param diriter The directory iterator
+ * @return 0 or an error code
+ */
+extern int git_path_diriter_filename(
 	const char **out,
 	size_t *out_len,
 	git_path_diriter *diriter);
 
+/**
+ * Returns the full path of the current item in the iterator; that
+ * is the current filename plus the path of the directory that the
+ * iterator was constructed with.
+ *
+ * @param out Pointer to store the path in
+ * @param out_len Pointer to store the length of the path in
+ * @param diriter The directory iterator
+ * @return 0 or an error code
+ */
 extern int git_path_diriter_fullpath(
 	const char **out,
 	size_t *out_len,
 	git_path_diriter *diriter);
 
+/**
+ * Performs an `lstat` on the current item in the iterator.
+ *
+ * @param out Pointer to store the stat data in
+ * @param diriter The directory iterator
+ * @return 0 or an error code
+ */
 extern int git_path_diriter_stat(struct stat *out, git_path_diriter *diriter);
 
+/**
+ * Closes the directory iterator.
+ *
+ * @param diriter The directory iterator
+ */
 extern void git_path_diriter_free(git_path_diriter *diriter);
 
 /**
diff --git a/tests/core/dirent.c b/tests/core/dirent.c
index f172603..d95e441 100644
--- a/tests/core/dirent.c
+++ b/tests/core/dirent.c
@@ -67,10 +67,23 @@ static void check_counts(walk_data *d)
 	}
 }
 
+static int update_count(name_data *data, const char *name)
+{
+	name_data *n;
+
+	for (n = data; n->name; n++) {
+		if (!strcmp(n->name, name)) {
+			n->count++;
+			return 0;
+		}
+	}
+
+	return GIT_ERROR;
+}
+
 static int one_entry(void *state, git_buf *path)
 {
 	walk_data *d = (walk_data *) state;
-	name_data *n;
 
 	if (state != state_loc)
 		return GIT_ERROR;
@@ -78,14 +91,7 @@ static int one_entry(void *state, git_buf *path)
 	if (path != &d->path)
 		return GIT_ERROR;
 
-	for (n = d->names; n->name; n++) {
-		if (!strcmp(n->name, path->ptr)) {
-			n->count++;
-			return 0;
-		}
-	}
-
-	return GIT_ERROR;
+	return update_count(d->names, path->ptr);
 }
 
 
@@ -234,3 +240,38 @@ void test_core_dirent__empty_dir(void)
 
 	cl_must_pass(p_rmdir("empty_dir"));
 }
+
+static void handle_next(git_path_diriter *diriter, walk_data *walk)
+{
+	const char *fullpath, *filename;
+	size_t fullpath_len, filename_len;
+
+	cl_git_pass(git_path_diriter_fullpath(&fullpath, &fullpath_len, diriter));
+	cl_git_pass(git_path_diriter_filename(&filename, &filename_len, diriter));
+
+	cl_assert_equal_strn(fullpath, "sub/", 4);
+	cl_assert_equal_s(fullpath+4, filename);
+
+	update_count(walk->names, fullpath);
+}
+
+/* test directory iterator */
+void test_core_dirent__diriter_with_fullname(void)
+{
+	git_path_diriter diriter = GIT_PATH_DIRITER_INIT;
+	int error;
+
+	cl_set_cleanup(&dirent_cleanup__cb, &sub);
+	setup(&sub);
+
+	cl_git_pass(git_path_diriter_init(&diriter, sub.path.ptr, 0));
+
+	while ((error = git_path_diriter_next(&diriter)) == 0)
+		handle_next(&diriter, &sub);
+
+	cl_assert_equal_i(error, GIT_ITEROVER);
+
+	git_path_diriter_free(&diriter);
+
+	check_counts(&sub);
+}