From: Thomas Rast <trast@student.ethz.ch>
To: <git@vger.kernel.org>
Cc: Junio C Hamano <gitster@pobox.com>, <desarrollo@gestiweb.com>,
<deavidsedice@gmail.com>
Subject: [PATCH] combined diff: correctly handle truncated file
Date: Thu, 15 Apr 2010 14:59:37 +0200 [thread overview]
Message-ID: <884b9b68a4478aceda580299c059a9a67417cb1c.1271336073.git.trast@student.ethz.ch> (raw)
In-Reply-To: <4BC6EECE.6060408@gestiweb.com>
Consider an evil merge of two commits A and B, both of which have a
file 'foo', but the merge result does not have that file.
The combined-diff code learned in 4462731 (combine-diff: do not punt
on removed or added files., 2006-02-06) to concisely show only the
removal, since that is the evil part and the previous contents are
presumably uninteresting.
However, to diagnose an empty merge result, it overloaded the variable
that holds the file's length. This means that the check also triggers
for truncated files. Consequently, such files were not shown in the
diff at all despite the merge being clearly evil.
Fix this by adding a new variable that distinguishes whether the file
was deleted (which is the case 4462731 handled) or truncated. In the
truncated case, we show the full combined diff again, which is rather
spammy but at least does not hide the evilness.
Reported-by: David Martínez Martí <desarrollo@gestiweb.com>
Signed-off-by: Thomas Rast <trast@student.ethz.ch>
---
combine-diff.c | 14 ++++++++------
t/t4038-diff-combined.sh | 8 ++++++++
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/combine-diff.c b/combine-diff.c
index 6162691..3480dae 100644
--- a/combine-diff.c
+++ b/combine-diff.c
@@ -204,7 +204,7 @@ static void consume_line(void *state_, char *line, unsigned long len)
static void combine_diff(const unsigned char *parent, unsigned int mode,
mmfile_t *result_file,
struct sline *sline, unsigned int cnt, int n,
- int num_parent)
+ int num_parent, int result_deleted)
{
unsigned int p_lno, lno;
unsigned long nmask = (1UL << n);
@@ -215,7 +215,7 @@ static void combine_diff(const unsigned char *parent, unsigned int mode,
struct combine_diff_state state;
unsigned long sz;
- if (!cnt)
+ if (result_deleted)
return; /* result deleted */
parent_file.ptr = grab_blob(parent, mode, &sz);
@@ -517,7 +517,7 @@ static void show_line_to_eol(const char *line, int len, const char *reset)
}
static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
- int use_color)
+ int use_color, int result_deleted)
{
unsigned long mark = (1UL<<num_parent);
unsigned long no_pre_delete = (2UL<<num_parent);
@@ -530,7 +530,7 @@ static void dump_sline(struct sline *sline, unsigned long cnt, int num_parent,
const char *c_plain = diff_get_color(use_color, DIFF_PLAIN);
const char *c_reset = diff_get_color(use_color, DIFF_RESET);
- if (!cnt)
+ if (result_deleted)
return; /* result deleted */
while (1) {
@@ -687,6 +687,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
{
struct diff_options *opt = &rev->diffopt;
unsigned long result_size, cnt, lno;
+ int result_deleted = 0;
char *result, *cp;
struct sline *sline; /* survived lines */
int mode_differs = 0;
@@ -767,6 +768,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
}
else {
deleted_file:
+ result_deleted = 1;
result_size = 0;
elem->mode = 0;
result = xcalloc(1, 1);
@@ -823,7 +825,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
combine_diff(elem->parent[i].sha1,
elem->parent[i].mode,
&result_file, sline,
- cnt, i, num_parent);
+ cnt, i, num_parent, result_deleted);
if (elem->parent[i].mode != elem->mode)
mode_differs = 1;
}
@@ -889,7 +891,7 @@ static void show_patch_diff(struct combine_diff_path *elem, int num_parent,
dump_quoted_path("+++ ", b_prefix, elem->path,
c_meta, c_reset);
dump_sline(sline, cnt, num_parent,
- DIFF_OPT_TST(opt, COLOR_DIFF));
+ DIFF_OPT_TST(opt, COLOR_DIFF), result_deleted);
}
free(result);
diff --git a/t/t4038-diff-combined.sh b/t/t4038-diff-combined.sh
index 7584efa..09e8c8a 100755
--- a/t/t4038-diff-combined.sh
+++ b/t/t4038-diff-combined.sh
@@ -81,4 +81,12 @@ test_expect_success 'check combined output (2)' '
verify_helper sidesansone
'
+test_expect_success 'diagnose truncated file' '
+ : > file &&
+ git add file &&
+ git commit --amend -C HEAD &&
+ git show > out &&
+ grep "diff --cc file" out
+'
+
test_done
--
1.7.1.rc1.265.g77471
next prev parent reply other threads:[~2010-04-15 12:59 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-04-15 10:47 Possible bug in Git David Martínez Martí
2010-04-15 12:59 ` Thomas Rast [this message]
2010-04-15 23:45 ` Avery Pennarun
2010-04-16 0:01 ` Junio C Hamano
2010-04-16 0:06 ` Avery Pennarun
2010-04-16 0:13 ` Avery Pennarun
2010-04-16 0:38 ` Jay Soffian
2010-04-16 0:45 ` Avery Pennarun
2010-04-16 15:53 ` Linus Torvalds
2010-04-16 16:39 ` Thomas Rast
2010-04-16 16:56 ` Junio C Hamano
2010-04-16 17:00 ` Thomas Rast
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=884b9b68a4478aceda580299c059a9a67417cb1c.1271336073.git.trast@student.ethz.ch \
--to=trast@student.ethz.ch \
--cc=deavidsedice@gmail.com \
--cc=desarrollo@gestiweb.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
/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).