From: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
To: git@vger.kernel.org
Cc: "Nguyễn Thái Ngọc Duy" <pclouds@gmail.com>
Subject: [PATCH 3/3] Correct conditions to free textconv result data
Date: Mon, 22 Feb 2016 19:52:25 +0700 [thread overview]
Message-ID: <1456145545-5374-3-git-send-email-pclouds@gmail.com> (raw)
In-Reply-To: <1456145545-5374-1-git-send-email-pclouds@gmail.com>
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(-)
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
next prev parent reply other threads:[~2016-02-22 12:52 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
[not found] <1456145545-5374-1-git-send-email-pclouds@gmail.com>
2016-02-22 12:52 ` [PATCH 2/3] diff.c: remove unnecessary typecast Nguyễn Thái Ngọc Duy
2016-02-22 12:52 ` Nguyễn Thái Ngọc Duy [this message]
2016-02-22 13:00 ` [PATCH 3/3] Correct conditions to free textconv result data Duy Nguyen
2016-02-22 18:06 ` Jeff King
2016-02-22 18:12 ` Jeff King
2016-02-22 23:03 ` Duy Nguyen
2016-02-22 23:08 ` Junio C Hamano
2016-02-22 23:25 ` Jeff King
2016-02-22 18:28 ` [PATCH] diff: clarify textconv interface Jeff King
2016-02-22 23:01 ` Duy Nguyen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1456145545-5374-3-git-send-email-pclouds@gmail.com \
--to=pclouds@gmail.com \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).