Commit 71d273583755c0a2b7f5d608f017f4586add51e4
2012-07-19T10:23:45
Fix bug with merging diffs with null options A diff that is created with a NULL options parameter could result in a NULL prefix string, but diff merge was unconditionally strdup'ing it. I added a test to replicate the issue and then a new method that does the right thing with NULL values.
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106
diff --git a/src/diff.c b/src/diff.c
index 4fea894..09f319e 100644
--- a/src/diff.c
+++ b/src/diff.c
@@ -814,9 +814,9 @@ int git_diff_merge(
/* prefix strings also come from old pool, so recreate those.*/
onto->opts.old_prefix =
- git_pool_strdup(&onto->pool, onto->opts.old_prefix);
+ git_pool_strdup_safe(&onto->pool, onto->opts.old_prefix);
onto->opts.new_prefix =
- git_pool_strdup(&onto->pool, onto->opts.new_prefix);
+ git_pool_strdup_safe(&onto->pool, onto->opts.new_prefix);
}
git_vector_foreach(&onto_new, i, delta)
diff --git a/src/pool.c b/src/pool.c
index 63bf09c..64b5c6b 100644
--- a/src/pool.c
+++ b/src/pool.c
@@ -206,6 +206,11 @@ char *git_pool_strdup(git_pool *pool, const char *str)
return git_pool_strndup(pool, str, strlen(str));
}
+char *git_pool_strdup_safe(git_pool *pool, const char *str)
+{
+ return str ? git_pool_strdup(pool, str) : NULL;
+}
+
char *git_pool_strcat(git_pool *pool, const char *a, const char *b)
{
void *ptr;
diff --git a/src/pool.h b/src/pool.h
index 54a2861..05d3392 100644
--- a/src/pool.h
+++ b/src/pool.h
@@ -90,6 +90,13 @@ extern char *git_pool_strndup(git_pool *pool, const char *str, size_t n);
extern char *git_pool_strdup(git_pool *pool, const char *str);
/**
+ * Allocate space and duplicate a string into it, NULL is no error.
+ *
+ * This is allowed only for pools with item_size == sizeof(char)
+ */
+extern char *git_pool_strdup_safe(git_pool *pool, const char *str);
+
+/**
* Allocate space for the concatenation of two strings.
*
* This is allowed only for pools with item_size == sizeof(char)
diff --git a/tests-clar/diff/tree.c b/tests-clar/diff/tree.c
index 4201ad2..be9eb6c 100644
--- a/tests-clar/diff/tree.c
+++ b/tests-clar/diff/tree.c
@@ -208,3 +208,51 @@ void test_diff_tree__bare(void)
git_tree_free(a);
git_tree_free(b);
}
+
+void test_diff_tree__merge(void)
+{
+ /* grabbed a couple of commit oids from the history of the attr repo */
+ const char *a_commit = "605812a";
+ const char *b_commit = "370fe9ec22";
+ const char *c_commit = "f5b0af1fb4f5c";
+ git_tree *a, *b, *c;
+ git_diff_list *diff1 = NULL, *diff2 = NULL;
+ diff_expects exp;
+
+ g_repo = cl_git_sandbox_init("attr");
+
+ cl_assert((a = resolve_commit_oid_to_tree(g_repo, a_commit)) != NULL);
+ cl_assert((b = resolve_commit_oid_to_tree(g_repo, b_commit)) != NULL);
+ cl_assert((c = resolve_commit_oid_to_tree(g_repo, c_commit)) != NULL);
+
+ cl_git_pass(git_diff_tree_to_tree(g_repo, NULL, a, b, &diff1));
+
+ cl_git_pass(git_diff_tree_to_tree(g_repo, NULL, c, b, &diff2));
+
+ git_tree_free(a);
+ git_tree_free(b);
+ git_tree_free(c);
+
+ cl_git_pass(git_diff_merge(diff1, diff2));
+
+ git_diff_list_free(diff2);
+
+ memset(&exp, 0, sizeof(exp));
+
+ cl_git_pass(git_diff_foreach(
+ diff1, &exp, diff_file_fn, diff_hunk_fn, diff_line_fn));
+
+ cl_assert(exp.files == 6);
+ cl_assert(exp.file_adds == 2);
+ cl_assert(exp.file_dels == 1);
+ cl_assert(exp.file_mods == 3);
+
+ cl_assert(exp.hunks == 6);
+
+ cl_assert(exp.lines == 59);
+ cl_assert(exp.line_ctxt == 1);
+ cl_assert(exp.line_adds == 36);
+ cl_assert(exp.line_dels == 22);
+
+ git_diff_list_free(diff1);
+}