Commit d85296ab9b9c4a01adb35d4d2438b72177aeabc4

Russell Belfer 2013-03-14T13:50:54

Fix valgrind issues (and mmap fallback for diff) This fixes a number of issues identified by valgrind - mostly missed free calls. Inside valgrind, mmap() may fail which causes some of the diff tests to fail. This adds a fallback code path to diff_output.c:get_workdir_content() where is the mmap() fails the code will now try to read the file data directly into allocated memory (which is what it would do if the data needed to be filtered anyhow).

diff --git a/src/checkout.c b/src/checkout.c
index 68ebbe3..e52649a 100644
--- a/src/checkout.c
+++ b/src/checkout.c
@@ -610,7 +610,7 @@ static int checkout_get_actions(
 		if (act & CHECKOUT_ACTION__CONFLICT)
 			counts[CHECKOUT_ACTION__CONFLICT]++;
 	}
-	
+
 	error = checkout_remaining_wd_items(data, workdir, wditem, &pathspec);
 	if (error < 0)
 		goto fail;
diff --git a/src/diff_output.c b/src/diff_output.c
index 43262b1..c0aff68 100644
--- a/src/diff_output.c
+++ b/src/diff_output.c
@@ -330,6 +330,33 @@ static int get_workdir_sm_content(
 	return 0;
 }
 
+static int get_filtered(
+	git_map *map, git_file fd, git_diff_file *file, git_vector *filters)
+{
+	int error;
+	git_buf raw = GIT_BUF_INIT, filtered = GIT_BUF_INIT;
+
+	if ((error = git_futils_readbuffer_fd(&raw, fd, (size_t)file->size)) < 0)
+		return error;
+
+	if (!filters->length)
+		git_buf_swap(&filtered, &raw);
+	else
+		error = git_filters_apply(&filtered, &raw, filters);
+
+	if (!error) {
+		map->len  = git_buf_len(&filtered);
+		map->data = git_buf_detach(&filtered);
+
+		file->flags |= GIT_DIFF_FLAG__FREE_DATA;
+	}
+
+	git_buf_free(&raw);
+	git_buf_free(&filtered);
+
+	return error;
+}
+
 static int get_workdir_content(
 	diff_context *ctxt,
 	git_diff_delta *delta,
@@ -381,8 +408,8 @@ static int get_workdir_content(
 			goto cleanup;
 		}
 
-		if (!file->size)
-			file->size = git_futils_filesize(fd);
+		if (!file->size && !(file->size = git_futils_filesize(fd)))
+			goto close_and_cleanup;
 
 		if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0 ||
 			(delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
@@ -394,26 +421,12 @@ static int get_workdir_content(
 			goto close_and_cleanup;
 
 		if (error == 0) { /* note: git_filters_load returns filter count */
-			if (!file->size)
-				goto close_and_cleanup;
-
 			error = git_futils_mmap_ro(map, fd, 0, (size_t)file->size);
-			file->flags |= GIT_DIFF_FLAG__UNMAP_DATA;
-		} else {
-			git_buf raw = GIT_BUF_INIT, filtered = GIT_BUF_INIT;
-
-			if (!(error = git_futils_readbuffer_fd(&raw, fd, (size_t)file->size)) &&
-				!(error = git_filters_apply(&filtered, &raw, &filters)))
-			{
-				map->len  = git_buf_len(&filtered);
-				map->data = git_buf_detach(&filtered);
-
-				file->flags |= GIT_DIFF_FLAG__FREE_DATA;
-			}
-
-			git_buf_free(&raw);
-			git_buf_free(&filtered);
+			if (!error)
+				file->flags |= GIT_DIFF_FLAG__UNMAP_DATA;
 		}
+		if (error != 0)
+			error = get_filtered(map, fd, file, &filters);
 
 close_and_cleanup:
 		git_filters_free(&filters);
diff --git a/src/errors.c b/src/errors.c
index c5f0b3b..e2629f6 100644
--- a/src/errors.c
+++ b/src/errors.c
@@ -103,6 +103,7 @@ int giterr_set_regex(const regex_t *regex, int error_code)
 
 void giterr_clear(void)
 {
+	set_error(0, NULL);
 	GIT_GLOBAL->last_error = NULL;
 
 	errno = 0;
diff --git a/tests-clar/repo/iterator.c b/tests-clar/repo/iterator.c
index 9e1f098..00c46d6 100644
--- a/tests-clar/repo/iterator.c
+++ b/tests-clar/repo/iterator.c
@@ -337,6 +337,8 @@ void test_repo_iterator__tree_icase(void)
 		&i, head, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
 	expect_iterator_items(i, 1, NULL, 6, NULL);
 	git_iterator_free(i);
+
+	git_tree_free(head);
 }
 
 void test_repo_iterator__tree_more(void)