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).
This commit is contained in:
Russell Belfer 2013-03-14 13:50:54 -07:00
parent 0c46863384
commit d85296ab9b
4 changed files with 37 additions and 21 deletions

View File

@ -330,6 +330,33 @@ static int get_workdir_sm_content(
return 0; 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( static int get_workdir_content(
diff_context *ctxt, diff_context *ctxt,
git_diff_delta *delta, git_diff_delta *delta,
@ -381,8 +408,8 @@ static int get_workdir_content(
goto cleanup; goto cleanup;
} }
if (!file->size) if (!file->size && !(file->size = git_futils_filesize(fd)))
file->size = git_futils_filesize(fd); goto close_and_cleanup;
if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0 || if ((error = diff_delta_is_binary_by_size(ctxt, delta, file)) < 0 ||
(delta->flags & GIT_DIFF_FLAG_BINARY) != 0) (delta->flags & GIT_DIFF_FLAG_BINARY) != 0)
@ -394,26 +421,12 @@ static int get_workdir_content(
goto close_and_cleanup; goto close_and_cleanup;
if (error == 0) { /* note: git_filters_load returns filter count */ 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); error = git_futils_mmap_ro(map, fd, 0, (size_t)file->size);
file->flags |= GIT_DIFF_FLAG__UNMAP_DATA; if (!error)
} else { file->flags |= GIT_DIFF_FLAG__UNMAP_DATA;
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 != 0)
error = get_filtered(map, fd, file, &filters);
close_and_cleanup: close_and_cleanup:
git_filters_free(&filters); git_filters_free(&filters);

View File

@ -103,6 +103,7 @@ int giterr_set_regex(const regex_t *regex, int error_code)
void giterr_clear(void) void giterr_clear(void)
{ {
set_error(0, NULL);
GIT_GLOBAL->last_error = NULL; GIT_GLOBAL->last_error = NULL;
errno = 0; errno = 0;

View File

@ -337,6 +337,8 @@ void test_repo_iterator__tree_icase(void)
&i, head, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z")); &i, head, flag | GIT_ITERATOR_DONT_AUTOEXPAND, "k", "k/Z"));
expect_iterator_items(i, 1, NULL, 6, NULL); expect_iterator_items(i, 1, NULL, 6, NULL);
git_iterator_free(i); git_iterator_free(i);
git_tree_free(head);
} }
void test_repo_iterator__tree_more(void) void test_repo_iterator__tree_more(void)