Skip to content

Commit

Permalink
Fix bug with merging diffs with null options
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
arrbee committed Jul 19, 2012
1 parent 5a8204f commit 71d2735
Show file tree
Hide file tree
Showing 4 changed files with 62 additions and 2 deletions.
4 changes: 2 additions & 2 deletions src/diff.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions src/pool.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
7 changes: 7 additions & 0 deletions src/pool.h
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,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.
*
Expand Down
48 changes: 48 additions & 0 deletions tests-clar/diff/tree.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

0 comments on commit 71d2735

Please sign in to comment.