Commit dedf70ad2b9f3dd5a5b299f64476285653cf62b7
2019-07-05T09:35:43
patch_parse: do not depend on parsed buffer's lifetime When parsing a patch from a buffer, we let the patch lines point into the original buffer. While this is efficient use of resources, this also ties the lifetime of the parsed patch to the parsed buffer. As this behaviour is not documented anywhere in our API it is very surprising to its users. Untie the lifetime by duplicating the lines into the parsed patch. Add a test that verifies that lifetimes are indeed independent of each other.
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
diff --git a/src/patch_parse.c b/src/patch_parse.c
index b44d4f0..5e5e5d9 100644
--- a/src/patch_parse.c
+++ b/src/patch_parse.c
@@ -588,8 +588,8 @@ static int parse_hunk_body(
memset(line, 0x0, sizeof(git_diff_line));
- line->content = ctx->parse_ctx.line + prefix;
line->content_len = ctx->parse_ctx.line_len - prefix;
+ line->content = git__strndup(ctx->parse_ctx.line + prefix, line->content_len);
line->content_offset = ctx->parse_ctx.content_len - ctx->parse_ctx.remain_len;
line->origin = origin;
line->num_lines = 1;
@@ -1038,6 +1038,8 @@ int git_patch_parsed_from_diff(git_patch **out, git_diff *d, size_t idx)
static void patch_parsed__free(git_patch *p)
{
git_patch_parsed *patch = (git_patch_parsed *)p;
+ git_diff_line *line;
+ size_t i;
if (!patch)
return;
@@ -1047,6 +1049,8 @@ static void patch_parsed__free(git_patch *p)
git__free((char *)patch->base.binary.old_file.data);
git__free((char *)patch->base.binary.new_file.data);
git_array_clear(patch->base.hunks);
+ git_array_foreach(patch->base.lines, i, line)
+ git__free((char *) line->content);
git_array_clear(patch->base.lines);
git__free(patch->base.delta);
diff --git a/tests/patch/parse.c b/tests/patch/parse.c
index a40ad7b..7eb9879 100644
--- a/tests/patch/parse.c
+++ b/tests/patch/parse.c
@@ -108,3 +108,23 @@ void test_patch_parse__files_with_whitespaces_succeeds(void)
cl_git_pass(git_patch_from_buffer(&patch, PATCH_NAME_WHITESPACE, strlen(PATCH_NAME_WHITESPACE), NULL));
git_patch_free(patch);
}
+
+void test_patch_parse__lifetime_of_patch_does_not_depend_on_buffer(void)
+{
+ git_buf diff = GIT_BUF_INIT, rendered = GIT_BUF_INIT;
+ git_patch *patch;
+
+ cl_git_pass(git_buf_sets(&diff, PATCH_ORIGINAL_TO_CHANGE_MIDDLE));
+ cl_git_pass(git_patch_from_buffer(&patch, diff.ptr, diff.size, NULL));
+ git_buf_dispose(&diff);
+
+ cl_git_pass(git_patch_to_buf(&rendered, patch));
+ cl_assert_equal_s(PATCH_ORIGINAL_TO_CHANGE_MIDDLE, rendered.ptr);
+ git_buf_dispose(&rendered);
+
+ cl_git_pass(git_patch_to_buf(&rendered, patch));
+ cl_assert_equal_s(PATCH_ORIGINAL_TO_CHANGE_MIDDLE, rendered.ptr);
+ git_buf_dispose(&rendered);
+
+ git_patch_free(patch);
+}