{"thread":{"id":"35831","subject":"[PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno","startedAt":"2014-02-08T09:19:26Z","lastAt":"2014-02-08T21:34:42Z","messageCount":4,"participants":["David Kastrup","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"234500","messageId":"1391851166-10393-1-git-send-email-dak@gnu.org","threadId":"35831","inReplyTo":null,"subject":"[PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-08T09:19:26Z","receivedAt":"2014-02-08T09:19:26Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"If we are calling xrealloc on every single line, the least we can do\nis get the right allocation size.\n\nSigned-off-by: David Kastrup \n---\nThis should be less contentious than the patch in\n,\nMessage-ID: <1391550392-17118-1-git-send-email-dak@gnu.org> as it\nmakes no stylistic decisions whatsoever and only fixes a clear bug.\n\nbuiltin/blame.c | 4 ++--\n 1 file changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/blame.c b/builtin/blame.c\nindex e44a6bb..29eb31c 100644\n--- a/builtin/blame.c\n+++ b/builtin/blame.c\n@@ -1779,7 +1779,7 @@ static int prepare_lines(struct scoreboard *sb)\n \twhile (len--) {\n \t\tif (bol) {\n \t\t\tsb->lineno = xrealloc(sb->lineno,\n-\t\t\t\t\t sizeof(int *) * (num + 1));\n+\t\t\t\t\t sizeof(int) * (num + 1));\n \t\t\tsb->lineno[num] = buf - sb->final_buf;\n \t\t\tbol = 0;\n \t\t}\n@@ -1789,7 +1789,7 @@ static int prepare_lines(struct scoreboard *sb)\n \t\t}\n \t}\n \tsb->lineno = xrealloc(sb->lineno,\n-\t\t\t sizeof(int *) * (num + incomplete + 1));\n+\t\t\t sizeof(int) * (num + incomplete + 1));\n \tsb->lineno[num + incomplete] = buf - sb->final_buf;\n \tsb->num_lines = num + incomplete;\n \treturn sb->num_lines;\n-- \n1.8.3.2\n"},{"id":"234501","messageId":"87lhxmc4sr.fsf@fencepost.gnu.org","threadId":"35831","inReplyTo":"1391851166-10393-1-git-send-email-dak@gnu.org","subject":"Re: [PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-08T09:49:40Z","receivedAt":"2014-02-08T09:49:40Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"David Kastrup writes:\n\n> If we are calling xrealloc on every single line, the least we can do\n> is get the right allocation size.\n>\n> Signed-off-by: David Kastrup \n> ---\n> This should be less contentious than the patch in\n> ,\n> Message-ID: <1391550392-17118-1-git-send-email-dak@gnu.org> as it\n> makes no stylistic decisions whatsoever and only fixes a clear bug.\n>\n> builtin/blame.c | 4 ++--\n> 1 file changed, 2 insertions(+), 2 deletions(-)\n>\n> diff --git a/builtin/blame.c b/builtin/blame.c\n> index e44a6bb..29eb31c 100644\n> --- a/builtin/blame.c\n> +++ b/builtin/blame.c\n> @@ -1779,7 +1779,7 @@ static int prepare_lines(struct scoreboard *sb)\n> \twhile (len--) {\n> \t\tif (bol) {\n> \t\t\tsb->lineno = xrealloc(sb->lineno,\n> -\t\t\t\t\t sizeof(int *) * (num + 1));\n> +\t\t\t\t\t sizeof(int) * (num + 1));\n\nBut please note that since sb->lineno originally comes from a zeroed\nmemory area and is passed to xrealloc, this requires that after\n\nint *p;\nmemset(&p, 0, sizeof(p));\n\nthe equivalence\n\n((void *)p == NULL)\n\nwill hold. While this is true on most platforms, and while the C\nstandard guarantees the slightly different\n((void *)0 == NULL)\nis true, it makes no statement concerning the memory representation of\nthe NULL pointer.\n\nI have not bothered addressing this non-compliance with the C standard\nas it would be polishing a turd. A wholesale replacement has already\nbeen proposed, and it's likely that this assumption is prevalent in the\nGit codebase elsewhere anyway.\n\n-- \nDavid Kastrup\n"},{"id":"234529","messageId":"20140208212154.GA4283@sigill.intra.peff.net","threadId":"35831","inReplyTo":"87lhxmc4sr.fsf@fencepost.gnu.org","subject":"Re: [PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2014-02-08T21:21:54Z","receivedAt":"2014-02-08T21:21:54Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sat, Feb 08, 2014 at 10:49:40AM +0100, David Kastrup wrote:\n\n> But please note that since sb->lineno originally comes from a zeroed\n> memory area and is passed to xrealloc, this requires that after\n> \n> int *p;\n> memset(&p, 0, sizeof(p));\n> \n> the equivalence\n> \n> ((void *)p == NULL)\n> \n> will hold. While this is true on most platforms, and while the C\n> standard guarantees the slightly different\n> ((void *)0 == NULL)\n> is true, it makes no statement concerning the memory representation of\n> the NULL pointer.\n> \n> I have not bothered addressing this non-compliance with the C standard\n> as it would be polishing a turd. A wholesale replacement has already\n> been proposed, and it's likely that this assumption is prevalent in the\n> Git codebase elsewhere anyway.\n\nYes, we explicitly break this part of the standard in the name of\npracticality (it simplifies frequently-used code, and machines on which\nit matters are rare enough that nobody has ever complained about it).\n\nSo I do not think this is a problem.\n\nHowever, is there a reason not to use:\n\n sizeof(*sb->lineno)\n\nrather than\n\n sizeof(int)\n\nto avoid type-mismatch errors entirely (this applies both to this patch,\nand to any proposed rewrites using malloc).\n\n-Peff\n"},{"id":"234532","messageId":"87ha89b85p.fsf@fencepost.gnu.org","threadId":"35831","inReplyTo":"20140208212154.GA4283@sigill.intra.peff.net","subject":"Re: [PATCH] builtin/blame.c::prepare_lines: fix allocation size of sb->lineno","fromName":"David Kastrup","fromEmail":"dak@gnu.org","sentAt":"2014-02-08T21:34:42Z","receivedAt":"2014-02-08T21:34:42Z","isPatch":true,"sender":{"key":"dak@gnu.org","avatar":"https://avatars.githubusercontent.com/u/52141349?v=4"},"body":"Jeff King writes:\n\n> However, is there a reason not to use:\n>\n> sizeof(*sb->lineno)\n>\n> rather than\n>\n> sizeof(int)\n>\n> to avoid type-mismatch errors entirely (this applies both to this patch,\n> and to any proposed rewrites using malloc).\n\nIt deviates from the style of the original code by tried and true Git\ndevelopers. So feel free to roll your own patch here: it's not like\nthis one has any copyrightable content in it.\n\n-- \nDavid Kastrup\n"}]}