[PATCH 3/3] Correct conditions to free textconv result data
fill_textconv() have four code paths to return a new buffer. Two of them, run_textconv() and notes_cache_get(), return a newly allocated buffer. The other two return either a constant string or an existing buffer (mmfile_t). We can only free the result buffer if it's allocated by fill_textconv(). Correct all call sites.
Changes in combine-diff.c are not clear from diff output. We need to force not running fill_textconv() unless the function returns a new buffer so that it falls to the "else" case and allocates a new buffer with no conversion.
Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
---
Frankly I don't like the way fill_textconv() returns buffer at all.
But I don't know much about textconv, or want to spend more time on
it. This patch can be dropped if people come up with a better one.
builtin/blame.c | 2 +-
combine-diff.c | 4 ++--
diff.c | 8 ++++----
diffcore-pickaxe.c | 5 +++--
4 files changed, 10 insertions(+), 9 deletions(-)
Show changes to 4 files +10 −9
builtin/blame.c, combine-diff.c, diff.c, diffcore-pickaxe.c
diff --git a/builtin/blame.c b/builtin/blame.c
index cb6f2fb..b5477ad 100644
--- a/builtin/blame.c
+++ b/builtin/blame.c
@@ -166,7 +166,7 @@ int textconv_object(const char *path,
df = alloc_filespec(path);
fill_filespec(df, sha1, sha1_valid, mode);
textconv = get_textconv(df);
- if (!textconv) {
+ if (!textconv || !textconv->textconv) {
free_filespec(df);
return 0;
}
diff --git a/combine-diff.c b/combine-diff.c
index 5571304..c57cefd 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -299,7 +299,7 @@ static char *grab_blob(const struct object_id *oid, unsigned int mode,
/* deleted blob */
*size = 0;
return xcalloc(1, 1);
- } else if (textconv) {
+ } else if (textconv && textconv->textconv) {
struct diff_filespec *df = alloc_filespec(path);
fill_filespec(df, oid->hash, 1, mode);
*size = fill_textconv(textconv, df, &blob);
@@ -1022,7 +1022,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
else
result = grab_blob(&oid, elem->mode,
&result_size, NULL, NULL);
- } else if (textconv) {
+ } else if (textconv && textconv->textconv) {
struct diff_filespec *df = alloc_filespec(elem->path);
fill_filespec(df, null_sha1, 0, st.st_mode);
result_size = fill_textconv(textconv, df, &result);
diff --git a/diff.c b/diff.c
index 5bdc3c0..173ec5b 100644
--- a/diff.c
+++ b/diff.c
@@ -744,9 +744,9 @@ static void emit_rewrite_diff(const char *name_a,
emit_rewrite_lines(&ecbdata, '-', data_one, size_one);
if (lc_b)
emit_rewrite_lines(&ecbdata, '+', data_two, size_two);
- if (textconv_one)
+ if (textconv_one && textconv_one->textconv)
free((char *)data_one);
- if (textconv_two)
+ if (textconv_two && textconv_two->textconv)
free((char *)data_two);
}
@@ -2454,9 +2454,9 @@ static void builtin_diff(const char *name_a,
die("unable to generate diff for %s", one->path);
if (o->word_diff)
free_diff_words_data(&ecbdata);
- if (textconv_one)
+ if (textconv_one && textconv_one->textconv)
free(mf1.ptr);
- if (textconv_two)
+ if (textconv_two && textconv_two->textconv)
free(mf2.ptr);
xdiff_clear_find_func(&xecfg);
}
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index 7715c13..f89e106 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -7,6 +7,7 @@
#include "diffcore.h"
#include "xdiff-interface.h"
#include "kwset.h"
+#include "userdiff.h"
typedef int (*pickaxe_fn)(mmfile_t *one, mmfile_t *two,
struct diff_options *o,
@@ -150,9 +151,9 @@ static int pickaxe_match(struct diff_filepair *p, struct diff_options *o,
DIFF_FILE_VALID(p->two) ? &mf2 : NULL,
o, regexp, kws);
- if (textconv_one)
+ if (textconv_one && textconv_one->textconv)
free(mf1.ptr);
- if (textconv_two)
+ if (textconv_two && textconv_two->textconv)
free(mf2.ptr);
diff_free_filespec_data(p->one);
diff_free_filespec_data(p->two);
--
2.7.1.532.gd9e3aaa
Nguyễn Thái Ngọc Duy
