Commit a521f5b164bfa513c56f9fd84647f840534e3827

Patrick Steinhardt 2017-12-15T10:47:01

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.

diff --git a/src/diff_file.c b/src/diff_file.c
index d5fc5e9..0d4201f 100644
--- a/src/diff_file.c
+++ b/src/diff_file.c
@@ -138,7 +138,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;
@@ -148,12 +147,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 c3933c3..05cc282 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;