fixed minor issues with new note iterator

* fixed style issues
* use new iterator functions for git_note_foreach()

4 files changed, 49 insertions, 62 deletions

diff --git a/include/git2/notes.h b/include/git2/notes.h
index be69c26ec..7382904ad 100644
--- a/include/git2/notes.h
+++ b/include/git2/notes.h
@@ -59,7 +59,8 @@ GIT_EXTERN(int) git_note_iterator_new(
 GIT_EXTERN(void) git_note_iterator_free(git_note_iterator *it);
 
 /**
- * Next iteration step for note iteration
+ * Returns the current item (note_id and annotated_id) and advance the iterator
+ * internally to the next value
  *
  * The notes must not be freed manually by the user.
  *
@@ -67,7 +68,8 @@ GIT_EXTERN(void) git_note_iterator_free(git_note_iterator *it);
  * @param note_id id of blob containing the message
  * @param annotated_id id of the git object being annotated
  *
- * @return 0, GIT_ITEROVER or an error code
+ * @return 0 (no error), GIT_ITEROVER (iteration is done) or an error code
+ *         (negative value)
  */
 GIT_EXTERN(int) git_note_next(
 	git_oid* note_id,
diff --git a/src/notes.c b/src/notes.c
index 987c5a6a3..0b286a77c 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -531,8 +531,7 @@ void git_note_free(git_note *note)
 
 static int process_entry_path(
 	const char* entry_path,
-	git_oid *annotated_object_id
-)
+	git_oid *annotated_object_id)
 {
 	int error = -1;
 	size_t i = 0, j = 0, len;
@@ -569,8 +568,7 @@ static int process_entry_path(
 		goto cleanup;
 	}
 
-	if ((error = git_oid_fromstr(annotated_object_id, buf.ptr)) < 0)
-		goto cleanup;
+	error = git_oid_fromstr(annotated_object_id, buf.ptr);
 
 cleanup:
 	git_buf_free(&buf);
@@ -578,40 +576,33 @@ cleanup:
 }
 
 int git_note_foreach(
-	git_repository *repo,
-	const char *notes_ref,
-	git_note_foreach_cb note_cb,
-	void *payload)
+    git_repository *repo,
+    const char *notes_ref,
+    git_note_foreach_cb note_cb,
+    void *payload)
 {
-	int error;
-	git_note_iterator *iter = NULL;
-	git_tree *tree = NULL;
-	git_commit *commit = NULL;
-	git_oid annotated_object_id;
-	const git_index_entry *item;
-
-	if (!(error = retrieve_note_tree_and_commit(
-			&tree, &commit, repo, &notes_ref)) &&
-		!(error = git_iterator_for_tree(&iter, tree)))
-		error = git_iterator_current(iter, &item);
-
-	while (!error && item) {
-		error = process_entry_path(item->path, &annotated_object_id);
+    int error;
+    git_note_iterator *iter = NULL;
+    git_oid note_id, annotated_id;
 
-		if (note_cb(&item->oid, &annotated_object_id, payload))
-			error = GIT_EUSER;
+    if ((error = git_note_iterator_new(&iter, repo, notes_ref)) < 0)
+        return error;
 
-		if (!error)
-			error = git_iterator_advance(iter, &item);
-	}
+    while (!(error = git_note_next(&note_id, &annotated_id, iter))) {
+        if (note_cb(&note_id, &annotated_id, payload)) {
+            error = GIT_EUSER;
+            break;
+        }
+    }
 
-	git_iterator_free(iter);
-	git_tree_free(tree);
-	git_commit_free(commit);
+    if (error == GIT_ITEROVER)
+        error = 0;
 
-	return error;
+    git_note_iterator_free(iter);
+    return error;
 }
 
+
 void git_note_iterator_free(git_note_iterator *it)
 {
 	if (it == NULL)
@@ -624,23 +615,20 @@ void git_note_iterator_free(git_note_iterator *it)
 int git_note_iterator_new(
 	git_note_iterator **it,
 	git_repository *repo,
-	const char *notes_ref
-)
+	const char *notes_ref)
 {
 	int error;
 	git_commit *commit = NULL;
 	git_tree *tree = NULL;
 
 	error = retrieve_note_tree_and_commit(&tree, &commit, repo, &notes_ref);
-	if (!error) {
-		*it = (git_note_iterator *)git__malloc(sizeof(git_iterator));
-		GITERR_CHECK_ALLOC(*it);
+	if (error < 0)
+		goto cleanup;
 
-		error = git_iterator_for_tree(it, tree);
-		if (error)
-			git_iterator_free(*it);
-	}
+	if ((error = git_iterator_for_tree(it, tree)) < 0)
+		git_iterator_free(*it);
 
+cleanup:
 	git_tree_free(tree);
 	git_commit_free(commit);
 
@@ -650,24 +638,24 @@ int git_note_iterator_new(
 int git_note_next(
 	git_oid* note_id,
 	git_oid* annotated_id,
-	git_note_iterator *it
-)
+	git_note_iterator *it)
 {
 	int error;
 	const git_index_entry *item;
 
-	error = git_iterator_current(it, &item);
-	if (!error && item) {
-		git_oid_cpy(note_id, &item->oid);
+	if (error = git_iterator_current(it, &item) < 0)
+		goto exit;
 
+	if (item != NULL) {
+		git_oid_cpy(note_id, &item->oid);
 		error = process_entry_path(item->path, annotated_id);
 
-		if (!error)
+		if (error >= 0)
 			error = git_iterator_advance(it, NULL);
-	}
-
-	if (!error && !item)
+	} else {
 		error = GIT_ITEROVER;
+	}
 
+exit:
 	return error;
 }
diff --git a/src/notes.h b/src/notes.h
index 70d5e13fc..39e18b621 100644
--- a/src/notes.h
+++ b/src/notes.h
@@ -12,8 +12,6 @@
 #include "git2/oid.h"
 #include "git2/types.h"
 
-#include "iterator.h"
-
 #define GIT_NOTES_DEFAULT_REF "refs/notes/commits"
 
 #define GIT_NOTES_DEFAULT_MSG_ADD \
diff --git a/tests-clar/notes/notes.c b/tests-clar/notes/notes.c
index b698648f7..40fa0db27 100644
--- a/tests-clar/notes/notes.c
+++ b/tests-clar/notes/notes.c
@@ -41,7 +41,6 @@ static struct {
 	const char *note_sha;
 	const char *annotated_object_sha;
 }
-
 list_expectations[] = {
 	{ "1c73b1f51762155d357bcd1fd4f2c409ef80065b", "4a202b346bb0fb0db7eff3cffeb3c70babbd2045" },
 	{ "1c73b1f51762155d357bcd1fd4f2c409ef80065b", "9fd738e8f7967c078dceed8190330fc8648ee56a" },
@@ -330,7 +329,7 @@ void test_notes_notes__can_iterate_default_namespace(void)
 		"I decorate a65f\n",
 		"I decorate c478\n"
 	};
-	int i;
+	int i, err;
 
 	create_note(&note_created[0], "refs/notes/commits",
 		"a65fedf39aefe402d3bb6e24df4d4f5fe4547750", note_message[0]);
@@ -339,13 +338,13 @@ void test_notes_notes__can_iterate_default_namespace(void)
 
 	cl_git_pass(git_note_iterator_new(&iter, _repo, NULL));
 
-	for (i = 0; git_note_next(&note_id, &annotated_id, iter) != GIT_ITEROVER; ++i)
-	{
+	for (i = 0; (err = git_note_next(&note_id, &annotated_id, iter)) >= 0; ++i) {
 		cl_git_pass(git_note_read(&note, _repo, NULL, &annotated_id));
 		cl_assert_equal_s(git_note_message(note), note_message[i]);
 	}
 
-	cl_assert(i == 2);
+	cl_assert_equal_i(GIT_ITEROVER, err);
+	cl_assert_equal_i(2, i);
 	git_note_iterator_free(iter);
 }
 
@@ -359,7 +358,7 @@ void test_notes_notes__can_iterate_custom_namespace(void)
 		"I decorate a65f\n",
 		"I decorate c478\n"
 	};
-	int i;
+	int i, err;
 
 	create_note(&note_created[0], "refs/notes/beer",
 		"a65fedf39aefe402d3bb6e24df4d4f5fe4547750", note_message[0]);
@@ -368,13 +367,13 @@ void test_notes_notes__can_iterate_custom_namespace(void)
 
 	cl_git_pass(git_note_iterator_new(&iter, _repo, "refs/notes/beer"));
 
-	for (i = 0; git_note_next(&note_id, &annotated_id, iter) != GIT_ITEROVER; ++i)
-	{
+	for (i = 0; (err = git_note_next(&note_id, &annotated_id, iter)) >= 0; ++i) {
 		cl_git_pass(git_note_read(&note, _repo, "refs/notes/beer", &annotated_id));
 		cl_assert_equal_s(git_note_message(note), note_message[i]);
 	}
 
-	cl_assert(i == 2);
+	cl_assert_equal_i(GIT_ITEROVER, err);
+	cl_assert_equal_i(2, i);
 	git_note_iterator_free(iter);
 }