Subject: [PATCH 3/3] Correct conditions to free textconv result data

From: Nguyễn Thái Ngọc Duy <[email-masked]>
Date: Mon, 22 Feb 2016 19:52:25 +0700

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 <[email-masked]>
---
 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(-)

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