From d85296ab9b9c4a01adb35d4d2438b72177aeabc4 Mon Sep 17 00:00:00 2001 From: Russell Belfer Date: Thu, 14 Mar 2013 13:50:54 -0700 Subject: [PATCH] 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). --- src/checkout.c | 2 +- src/diff_output.c | 53 ++++++++++++++++++++++++-------------- src/errors.c | 1 + tests-clar/repo/iterator.c | 2 ++ 4 files changed, 37 insertions(+), 21 deletions(-) diff --git a/src/checkout.c b/src/checkout.c index 68ebbe31d..e52649aec 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 43262b1ae..c0aff6826 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 c5f0b3b59..e2629f69e 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 9e1f09881..00c46d6b1 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)