Commit aba6b5edbd1d1f999086669eb8e2f553af0ac300

Russell Belfer 2014-03-14T21:59:26

Fix leak in git_index_conflict_cleanup I introduced a leak into conflict cleanup by removing items from inside the git_vector_remove_matching call. This simplifies the code to just use one common way for the two conflict cleanup APIs. When an index has an active snapshot, removing an item can cause an error (inserting into the deferred deletion vector), so I made the git_index_conflict_cleanup API return an error code. I felt like this wasn't so bad since it is just like the other APIs. I fixed up a couple of comments while I was changing the header.

diff --git a/include/git2/index.h b/include/git2/index.h
index 4d33f13..05e58a6 100644
--- a/include/git2/index.h
+++ b/include/git2/index.h
@@ -575,8 +575,7 @@ GIT_EXTERN(int) git_index_update_all(
  * @param at_pos the address to which the position of the index entry is written (optional)
  * @param index an existing index object
  * @param path path to search
- * @return a zero-based position in the index if found;
- * GIT_ENOTFOUND otherwise
+ * @return a zero-based position in the index if found; GIT_ENOTFOUND otherwise
  */
 GIT_EXTERN(int) git_index_find(size_t *at_pos, git_index *index, const char *path);
 
@@ -620,6 +619,7 @@ GIT_EXTERN(int) git_index_conflict_add(
  * @param their_out Pointer to store the their entry
  * @param index an existing index object
  * @param path path to search
+ * @return 0 or an error code
  */
 GIT_EXTERN(int) git_index_conflict_get(
 	const git_index_entry **ancestor_out,
@@ -632,16 +632,18 @@ GIT_EXTERN(int) git_index_conflict_get(
  * Removes the index entries that represent a conflict of a single file.
  *
  * @param index an existing index object
- * @param path to search
+ * @param path path to remove conflicts for
+ * @return 0 or an error code
  */
 GIT_EXTERN(int) git_index_conflict_remove(git_index *index, const char *path);
 
 /**
- * Remove all conflicts in the index (entries with a stage greater than 0.)
+ * Remove all conflicts in the index (entries with a stage greater than 0).
  *
  * @param index an existing index object
+ * @return 0 or an error code
  */
-GIT_EXTERN(void) git_index_conflict_cleanup(git_index *index);
+GIT_EXTERN(int) git_index_conflict_cleanup(git_index *index);
 
 /**
  * Determine if the index contains entries representing file conflicts.
@@ -651,9 +653,12 @@ GIT_EXTERN(void) git_index_conflict_cleanup(git_index *index);
 GIT_EXTERN(int) git_index_has_conflicts(const git_index *index);
 
 /**
- * Create an iterator for the conflicts in the index.  You may not modify the
- * index while iterating, the results are undefined.
+ * Create an iterator for the conflicts in the index.
+ *
+ * The index must not be modified while iterating; the results are undefined.
  *
+ * @param iterator_out The newly created conflict iterator
+ * @param index The index to scan
  * @return 0 or an error code
  */
 GIT_EXTERN(int) git_index_conflict_iterator_new(
diff --git a/src/index.c b/src/index.c
index 10fb1fe..27a557c 100644
--- a/src/index.c
+++ b/src/index.c
@@ -1317,15 +1317,13 @@ int git_index_conflict_get(
 	return 0;
 }
 
-int git_index_conflict_remove(git_index *index, const char *path)
+static int index_conflict_remove(git_index *index, const char *path)
 {
-	size_t pos;
+	size_t pos = 0;
 	git_index_entry *conflict_entry;
 	int error = 0;
 
-	assert(index && path);
-
-	if (git_index_find(&pos, index, path) < 0)
+	if (path != NULL && git_index_find(&pos, index, path) < 0)
 		return GIT_ENOTFOUND;
 
 	if (git_mutex_lock(&index->lock) < 0) {
@@ -1335,7 +1333,8 @@ int git_index_conflict_remove(git_index *index, const char *path)
 
 	while ((conflict_entry = git_vector_get(&index->entries, pos)) != NULL) {
 
-		if (index->entries_cmp_path(conflict_entry->path, path) != 0)
+		if (path != NULL &&
+			index->entries_cmp_path(conflict_entry->path, path) != 0)
 			break;
 
 		if (GIT_IDXENTRY_STAGE(conflict_entry) == 0) {
@@ -1352,27 +1351,16 @@ int git_index_conflict_remove(git_index *index, const char *path)
 	return error;
 }
 
-static int index_conflicts_match(const git_vector *v, size_t idx, void *p)
+int git_index_conflict_remove(git_index *index, const char *path)
 {
-	git_index *index = p;
-	git_index_entry *entry = git_vector_get(v, idx);
-
-	if (GIT_IDXENTRY_STAGE(entry) > 0 &&
-		!index_remove_entry(index, idx))
-		return 1;
-
-	return 0;
+	assert(index && path);
+	return index_conflict_remove(index, path);
 }
 
-void git_index_conflict_cleanup(git_index *index)
+int git_index_conflict_cleanup(git_index *index)
 {
 	assert(index);
-
-	if (git_mutex_lock(&index->lock) < 0)
-		return;
-	git_vector_remove_matching(&index->entries, index_conflicts_match, index);
-	index_free_deleted(index);
-	git_mutex_unlock(&index->lock);
+	return index_conflict_remove(index, NULL);
 }
 
 int git_index_has_conflicts(const git_index *index)