aboutsummaryrefslogtreecommitdiff
path: root/diff.c
diff options
context:
space:
mode:
authorJeff King <peff@peff.net>2008-10-26 00:44:53 -0400
committerJunio C Hamano <gitster@pobox.com>2008-10-26 14:09:48 -0700
commit04427ac8483f61dcb01a48c78a821f5042c88195 (patch)
tree90c925a7bd0d756d86bfa092ca779b5834b5554f /diff.c
parentdf5e91fc2c95e051744ec9b9de66869d2b323037 (diff)
downloadgit-04427ac8483f61dcb01a48c78a821f5042c88195.tar.gz
git-04427ac8483f61dcb01a48c78a821f5042c88195.tar.xz
refactor userdiff textconv code
The original implementation of textconv put the conversion into fill_mmfile. This was a bad idea for a number of reasons: - it made the semantics of fill_mmfile unclear. In some cases, it was allocating data (if a text conversion occurred), and in some cases not (if we could use the data directly from the filespec). But the caller had no idea which had happened, and so didn't know whether the memory should be freed - similarly, the caller had no idea if a text conversion had occurred, and so didn't know whether the contents should be treated as binary or not. This meant that we incorrectly guessed that text-converted content was binary and didn't actually show it (unless the user overrode us with "diff.foo.binary = false", which then created problems in plumbing where the text conversion did _not_ occur) - not all callers of fill_mmfile want the text contents. In particular, we don't really want diffstat, whitespace checks, patch id generation, etc, to look at the converted contents. This patch pulls the conversion code directly into builtin_diff, so that we only see the conversion when generating an actual patch. We also then know whether we are doing a conversion, so we can check the binary-ness and free the data from the mmfile appropriately (the previous version leaked quite badly when text conversion was used) Signed-off-by: Jeff King <peff@peff.net> Signed-off-by: Junio C Hamano <gitster@pobox.com>
Diffstat (limited to 'diff.c')
-rw-r--r--diff.c48
1 files changed, 35 insertions, 13 deletions
diff --git a/diff.c b/diff.c
index d1fd594ba..6f01595ec 100644
--- a/diff.c
+++ b/diff.c
@@ -294,18 +294,8 @@ static int fill_mmfile(mmfile_t *mf, struct diff_filespec *one)
else if (diff_populate_filespec(one, 0))
return -1;
- diff_filespec_load_driver(one);
- if (one->driver->textconv) {
- size_t size;
- mf->ptr = run_textconv(one->driver->textconv, one, &size);
- if (!mf->ptr)
- return -1;
- mf->size = size;
- }
- else {
- mf->ptr = one->data;
- mf->size = one->size;
- }
+ mf->ptr = one->data;
+ mf->size = one->size;
return 0;
}
@@ -1323,6 +1313,14 @@ void diff_set_mnemonic_prefix(struct diff_options *options, const char *a, const
options->b_prefix = b;
}
+static const char *get_textconv(struct diff_filespec *one)
+{
+ if (!DIFF_FILE_VALID(one))
+ return NULL;
+ diff_filespec_load_driver(one);
+ return one->driver->textconv;
+}
+
static void builtin_diff(const char *name_a,
const char *name_b,
struct diff_filespec *one,
@@ -1337,6 +1335,7 @@ static void builtin_diff(const char *name_a,
const char *set = diff_get_color_opt(o, DIFF_METAINFO);
const char *reset = diff_get_color_opt(o, DIFF_RESET);
const char *a_prefix, *b_prefix;
+ const char *textconv_one, *textconv_two;
diff_set_mnemonic_prefix(o, "a/", "b/");
if (DIFF_OPT_TST(o, REVERSE_DIFF)) {
@@ -1390,8 +1389,12 @@ static void builtin_diff(const char *name_a,
if (fill_mmfile(&mf1, one) < 0 || fill_mmfile(&mf2, two) < 0)
die("unable to read files to diff");
+ textconv_one = get_textconv(one);
+ textconv_two = get_textconv(two);
+
if (!DIFF_OPT_TST(o, TEXT) &&
- (diff_filespec_is_binary(one) || diff_filespec_is_binary(two))) {
+ ( (diff_filespec_is_binary(one) && !textconv_one) ||
+ (diff_filespec_is_binary(two) && !textconv_two) )) {
/* Quite common confusing case */
if (mf1.size == mf2.size &&
!memcmp(mf1.ptr, mf2.ptr, mf1.size))
@@ -1412,6 +1415,21 @@ static void builtin_diff(const char *name_a,
struct emit_callback ecbdata;
const struct userdiff_funcname *pe;
+ if (textconv_one) {
+ size_t size;
+ mf1.ptr = run_textconv(textconv_one, one, &size);
+ if (!mf1.ptr)
+ die("unable to read files to diff");
+ mf1.size = size;
+ }
+ if (textconv_two) {
+ size_t size;
+ mf2.ptr = run_textconv(textconv_two, two, &size);
+ if (!mf2.ptr)
+ die("unable to read files to diff");
+ mf2.size = size;
+ }
+
pe = diff_funcname_pattern(one);
if (!pe)
pe = diff_funcname_pattern(two);
@@ -1443,6 +1461,10 @@ static void builtin_diff(const char *name_a,
&xpp, &xecfg, &ecb);
if (DIFF_OPT_TST(o, COLOR_DIFF_WORDS))
free_diff_words_data(&ecbdata);
+ if (textconv_one)
+ free(mf1.ptr);
+ if (textconv_two)
+ free(mf2.ptr);
}
free_ab_and_return: