{"thread":{"id":"6921","subject":"[PATCH] git-diff: fix combined diff","startedAt":"2007-02-23T04:20:32Z","lastAt":"2007-02-23T11:19:40Z","messageCount":5,"participants":["Johannes Schindelin","Junio C Hamano","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"35292","messageId":"Pine.LNX.4.63.0702230517480.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6921","inReplyTo":null,"subject":"[PATCH] git-diff: fix combined diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-23T04:20:32Z","receivedAt":"2007-02-23T04:20:32Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nWith \"const unsigned (*parent)[20]\", \"parent + 1\" is not the\nsame as \"&parent[1]\"...\n\nSigned-off-by: Johannes Schindelin \n---\n\n\tI triggered this bug by \"git diff a...b\", where a and b\n\thave 4 merge bases. I am really too tired to add a test\n\tcase, though...\n\n builtin-diff.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex 9334589..846cafb 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -158,7 +158,8 @@ static int builtin_diff_combined(struct rev_info *revs,\n \tparent = xmalloc(ents * sizeof(*parent));\n \t/* Again, the revs are all reverse */\n \tfor (i = 0; i < ents; i++)\n-\t\thashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1);\n+\t\thashcpy((unsigned char*)&parent[i],\n+\t\t\t\tent[ents - 1 - i].item->sha1);\n \tdiff_tree_combined(parent[0], parent + 1, ents - 1,\n \t\t\t revs->dense_combined_merges, revs);\n \treturn 0;\n-- \n1.5.0.1.2218.g2de79\n"},{"id":"35295","messageId":"7vbqjlbg8y.fsf@assigned-by-dhcp.cox.net","threadId":"6921","inReplyTo":"Pine.LNX.4.63.0702230517480.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-diff: fix combined diff","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-23T05:22:05Z","receivedAt":"2007-02-23T05:22:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"With this,\n\n\t$ git diff maint master next pu\n\nstarts working as planned ;-).\n\nThanks.\n"},{"id":"35296","messageId":"20070223053428.GA13301@coredump.intra.peff.net","threadId":"6921","inReplyTo":"Pine.LNX.4.63.0702230517480.22628@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] git-diff: fix combined diff","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2007-02-23T05:34:28Z","receivedAt":"2007-02-23T05:34:28Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Feb 23, 2007 at 05:20:32AM +0100, Johannes Schindelin wrote:\n\n> With \"const unsigned (*parent)[20]\", \"parent + 1\" is not the\n> same as \"&parent[1]\"...\n\nActually, they _are_ the same (the C standard definition of A[B] is *(A+B)).\nThe problem is the operator precedence of the cast:\n\n> -\t\thashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1);\n\nwhich translates to \"cast parent to an unsigned char pointer, and then\nadd i * sizeof(unsigned char) to it\".\n\nYour fix works because [] binds more tightly, fixing the precedence\nproblem. You could also do this:\n hashcpy((unsigned char*)(parent + i), ...\n\n-Peff\n"},{"id":"35298","messageId":"7v3b4xbe1i.fsf@assigned-by-dhcp.cox.net","threadId":"6921","inReplyTo":"7vbqjlbg8y.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] git-diff: fix combined diff","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2007-02-23T06:09:45Z","receivedAt":"2007-02-23T06:09:45Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\n> With this,\n>\n> \t$ git diff maint master next pu\n>\n> starts working as planned ;-).\n\nBy the way, your commit log message does not seem to say\nwhy it is a fix correctly. Here is what I replaced it with.\n\n-- >8 -- cut here -- >8 --\nFrom: Johannes Schindelin \nDate: Fri, 23 Feb 2007 05:20:32 +0100\nSubject: [PATCH] git-diff: fix combined diff\n\nThe code forgets that typecast binds tighter than addition, in\nother words:\n\n (cast *)array + i === ((cast *)array) + i\n\nSigned-off-by: Johannes Schindelin \nSigned-off-by: Junio C Hamano \n---\n builtin-diff.c | 3 ++-\n 1 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/builtin-diff.c b/builtin-diff.c\nindex a659020..c387ebb 100644\n--- a/builtin-diff.c\n+++ b/builtin-diff.c\n@@ -192,7 +192,8 @@ static int builtin_diff_combined(struct rev_info *revs,\n \tparent = xmalloc(ents * sizeof(*parent));\n \t/* Again, the revs are all reverse */\n \tfor (i = 0; i < ents; i++)\n-\t\thashcpy((unsigned char*)parent + i, ent[ents - 1 - i].item->sha1);\n+\t\thashcpy((unsigned char *)(parent + i),\n+\t\t\tent[ents - 1 - i].item->sha1);\n \tdiff_tree_combined(parent[0], parent + 1, ents - 1,\n \t\t\t revs->dense_combined_merges, revs);\n \treturn 0;\n-- \n1.5.0.1.619.g04c5c\n"},{"id":"35320","messageId":"Pine.LNX.4.63.0702231219080.22628@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"6921","inReplyTo":"20070223053428.GA13301@coredump.intra.peff.net","subject":"Re: [PATCH] git-diff: fix combined diff","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-02-23T11:19:40Z","receivedAt":"2007-02-23T11:19:40Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 23 Feb 2007, Jeff King wrote:\n\n> On Fri, Feb 23, 2007 at 05:20:32AM +0100, Johannes Schindelin wrote:\n> \n> > With \"const unsigned (*parent)[20]\", \"parent + 1\" is not the\n> > same as \"&parent[1]\"...\n> \n> Actually, they _are_ the same (the C standard definition of A[B] is *(A+B)).\n\nModulo binding precedence.\n\nThanks for clearing this up,\nDscho\n"}]}