# [PATCH] git-diff: fix combined diff 5 messages from 2007-02-23 to 2007-02-23. Participants: Johannes Schindelin, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/6921 ## Johannes Schindelin, 2007-02-23 04:20 Subject: [PATCH] git-diff: fix combined diff Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0702230517480.22628%40wbgn013.biozentrum.uni-wuerzburg.de ``` With "const unsigned (*parent)[20]", "parent + 1" is not the same as "&parent[1]"... Signed-off-by: Johannes Schindelin --- I triggered this bug by "git diff a...b", where a and b have 4 merge bases. I am really too tired to add a test case, though... builtin-diff.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/builtin-diff.c b/builtin-diff.c index 9334589..846cafb 100644 --- a/builtin-diff.c +++ b/builtin-diff.c @@ -158,7 +158,8 @@ static int builtin_diff_combined(struct rev_info *revs, parent = xmalloc(ents * sizeof(*parent)); /* Again, the revs are all reverse */ for (i = 0; i < ents; i++) - hashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1); + hashcpy((unsigned char*)&parent[i], + ent[ents - 1 - i].item->sha1); diff_tree_combined(parent[0], parent + 1, ents - 1, revs->dense_combined_merges, revs); return 0; -- 1.5.0.1.2218.g2de79 ``` ## Junio C Hamano, 2007-02-23 05:22 Subject: Re: [PATCH] git-diff: fix combined diff Message-ID: <7vbqjlbg8y.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7vbqjlbg8y.fsf%40assigned-by-dhcp.cox.net In-Reply-To: ``` With this, $ git diff maint master next pu starts working as planned ;-). Thanks. ``` ## Jeff King, 2007-02-23 05:34 Subject: Re: [PATCH] git-diff: fix combined diff Message-ID: <20070223053428.GA13301@coredump.intra.peff.net> URL: https://gitlist.dev/e/20070223053428.GA13301%40coredump.intra.peff.net In-Reply-To: ``` On Fri, Feb 23, 2007 at 05:20:32AM +0100, Johannes Schindelin wrote: > With "const unsigned (*parent)[20]", "parent + 1" is not the > same as "&parent[1]"... Actually, they _are_ the same (the C standard definition of A[B] is *(A+B)). The problem is the operator precedence of the cast: > - hashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1); which translates to "cast parent to an unsigned char pointer, and then add i * sizeof(unsigned char) to it". Your fix works because [] binds more tightly, fixing the precedence problem. You could also do this: hashcpy((unsigned char*)(parent + i), ... -Peff ``` ## Junio C Hamano, 2007-02-23 06:09 Subject: Re: [PATCH] git-diff: fix combined diff Message-ID: <7v3b4xbe1i.fsf@assigned-by-dhcp.cox.net> URL: https://gitlist.dev/e/7v3b4xbe1i.fsf%40assigned-by-dhcp.cox.net In-Reply-To: <7vbqjlbg8y.fsf@assigned-by-dhcp.cox.net> ``` Junio C Hamano writes: > With this, > > $ git diff maint master next pu > > starts working as planned ;-). By the way, your commit log message does not seem to say why it is a fix correctly. Here is what I replaced it with. -- >8 -- cut here -- >8 -- From: Johannes Schindelin Date: Fri, 23 Feb 2007 05:20:32 +0100 Subject: [PATCH] git-diff: fix combined diff The code forgets that typecast binds tighter than addition, in other words: (cast *)array + i === ((cast *)array) + i Signed-off-by: Johannes Schindelin Signed-off-by: Junio C Hamano --- builtin-diff.c | 3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/builtin-diff.c b/builtin-diff.c index a659020..c387ebb 100644 --- a/builtin-diff.c +++ b/builtin-diff.c @@ -192,7 +192,8 @@ static int builtin_diff_combined(struct rev_info *revs, parent = xmalloc(ents * sizeof(*parent)); /* Again, the revs are all reverse */ for (i = 0; i < ents; i++) - hashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1); + hashcpy((unsigned char *)(parent + i), + ent[ents - 1 - i].item->sha1); diff_tree_combined(parent[0], parent + 1, ents - 1, revs->dense_combined_merges, revs); return 0; -- 1.5.0.1.619.g04c5c ``` ## Johannes Schindelin, 2007-02-23 11:19 Subject: Re: [PATCH] git-diff: fix combined diff Message-ID: URL: https://gitlist.dev/e/Pine.LNX.4.63.0702231219080.22628%40wbgn013.biozentrum.uni-wuerzburg.de In-Reply-To: <20070223053428.GA13301@coredump.intra.peff.net> ``` Hi, On Fri, 23 Feb 2007, Jeff King wrote: > On Fri, Feb 23, 2007 at 05:20:32AM +0100, Johannes Schindelin wrote: > > > With "const unsigned (*parent)[20]", "parent + 1" is not the > > same as "&parent[1]"... > > Actually, they _are_ the same (the C standard definition of A[B] is *(A+B)). Modulo binding precedence. Thanks for clearing this up, Dscho ```