From 2388a9e2ab0516c2a9146a1c4d15ced3052fef4c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 15 Dec 2017 10:47:01 +0000 Subject: diff_file: properly refcount blobs when initializing file contents When initializing a `git_diff_file_content` from a source whose data is derived from a blob, we simply assign the blob's pointer to the resulting struct without incrementing its refcount. Thus, the structure can only be used as long as the blob is kept alive by the caller. Fix the issue by using `git_blob_dup` instead of a direct assignment. This function will increment the refcount of the blob without allocating new memory, so it does exactly what we want. As `git_diff_file_content__unload` already frees the blob when `GIT_DIFF_FLAG__FREE_BLOB` is set, we don't need to add new code handling the free but only have to set that flag correctly. --- src/diff_file.c | 4 +++- tests/diff/blob.c | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/src/diff_file.c b/src/diff_file.c index 270d59bbb..0813315f5 100644 --- a/src/diff_file.c +++ b/src/diff_file.c @@ -139,7 +139,6 @@ int git_diff_file_content__init_from_src( memset(fc, 0, sizeof(*fc)); fc->repo = repo; fc->file = as_file; - fc->blob = src->blob; if (!src->blob && !src->buf) { fc->flags |= GIT_DIFF_FLAG__NO_DATA; @@ -149,12 +148,15 @@ int git_diff_file_content__init_from_src( fc->file->mode = GIT_FILEMODE_BLOB; if (src->blob) { + git_blob_dup((git_blob **)&fc->blob, (git_blob *) src->blob); fc->file->size = git_blob_rawsize(src->blob); git_oid_cpy(&fc->file->id, git_blob_id(src->blob)); fc->file->id_abbrev = GIT_OID_HEXSZ; fc->map.len = (size_t)fc->file->size; fc->map.data = (char *)git_blob_rawcontent(src->blob); + + fc->flags |= GIT_DIFF_FLAG__FREE_BLOB; } else { fc->file->size = src->buflen; git_odb_hash(&fc->file->id, src->buf, src->buflen, GIT_OBJ_BLOB); diff --git a/tests/diff/blob.c b/tests/diff/blob.c index c3933c313..05cc28218 100644 --- a/tests/diff/blob.c +++ b/tests/diff/blob.c @@ -1,6 +1,19 @@ #include "clar_libgit2.h" #include "diff_helpers.h" +#define BLOB_DIFF \ + "diff --git a/file b/file\n" \ + "index 45141a7..4d713dc 100644\n" \ + "--- a/file\n" \ + "+++ b/file\n" \ + "@@ -1 +1,6 @@\n" \ + " Hello from the root\n" \ + "+\n" \ + "+Some additional lines\n" \ + "+\n" \ + "+Down here below\n" \ + "+\n" + static git_repository *g_repo = NULL; static diff_expects expected; static git_diff_options opts; @@ -65,6 +78,32 @@ static void assert_one_modified( cl_assert_equal_i(dels, exp->line_dels); } +void test_diff_blob__patch_with_freed_blobs(void) +{ + git_oid a_oid, b_oid; + git_blob *a, *b; + git_patch *p; + git_buf buf = GIT_BUF_INIT; + + /* tests/resources/attr/root_test1 */ + cl_git_pass(git_oid_fromstrn(&a_oid, "45141a79", 8)); + cl_git_pass(git_blob_lookup_prefix(&a, g_repo, &a_oid, 4)); + /* tests/resources/attr/root_test2 */ + cl_git_pass(git_oid_fromstrn(&b_oid, "4d713dc4", 8)); + cl_git_pass(git_blob_lookup_prefix(&b, g_repo, &b_oid, 4)); + + cl_git_pass(git_patch_from_blobs(&p, a, NULL, b, NULL, NULL)); + + git_blob_free(a); + git_blob_free(b); + + cl_git_pass(git_patch_to_buf(&buf, p)); + cl_assert_equal_s(buf.ptr, BLOB_DIFF); + + git_patch_free(p); + git_buf_free(&buf); +} + void test_diff_blob__can_compare_text_blobs(void) { git_blob *a, *b, *c; -- cgit v1.2.1