{"thread":{"id":"43213","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","startedAt":"2006-12-09T03:23:14Z","lastAt":"2006-12-10T22:43:19Z","messageCount":8,"participants":["Johannes Schindelin","Junio C Hamano","Nicolas Pitre","Jeff King"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"298100","messageId":"Pine.LNX.4.64.0612082205240.2630@xanadu.home","threadId":"43213","inReplyTo":null,"subject":"builtin git-shortlog still broken","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-09T03:23:14Z","receivedAt":"2006-12-09T03:23:14Z","isPatch":false,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On the Linux kernel repository, doing \"git shortlog v2.6.18..\" works \nfine. \"git shortlog v2.6.17..\" works fine. \"git shortlog v2.6.16..\" also \nworks fine. But \"git shortlog v2.6.15..\" dies with a segmentation \nfault. Trying \"git log v2.6.15.. | git shortlog\" also produces the same \ncrash while \"git log v2.6.16.. | git shortlog\" works fine.\n\nThe old perl version doesn't have any such issue with those test cases, \nnot even with the whole kernel history.\n\n\n"},{"id":"298141","messageId":"20061209040421.GA29113@coredump.intra.peff.net","threadId":"43213","inReplyTo":"Pine.LNX.4.64.0612082205240.2630@xanadu.home","subject":"[PATCH] shortlog: fix segfault on empty authorname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-12-09T04:04:21Z","receivedAt":"2006-12-09T04:04:21Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"The old code looked backwards from the email address to parse the name,\nallowing an arbitrary number of spaces between the two. However, in the case\nof no name, we looked back too far to the 'author' (or 'Author:') header.\nInstead, remove at most one space between name and address.\n\nThe bug was triggered by commit febf7ea4bed from linux-2.6.\n\nSigned-off-by: Jeff King \n---\n builtin-shortlog.c | 4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin-shortlog.c b/builtin-shortlog.c\nindex f1124e2..7a2ddfe 100644\n--- a/builtin-shortlog.c\n+++ b/builtin-shortlog.c\n@@ -188,7 +188,7 @@ static void read_from_stdin(struct path_list *list)\n \t\t\t\tbob = buffer + strlen(buffer);\n \t\t\telse {\n \t\t\t\toffset = 8;\n-\t\t\t\twhile (isspace(bob[-1]))\n+\t\t\t\tif (isspace(bob[-1]))\n \t\t\t\t\tbob--;\n \t\t\t}\n \n@@ -236,7 +236,7 @@ static void get_from_rev(struct rev_info *rev, struct path_list *list)\n \t\t\t\t\tauthor = scratch;\n \t\t\t\t\tauthorlen = strlen(scratch);\n \t\t\t\t} else {\n-\t\t\t\t\twhile (bracket[-1] == ' ')\n+\t\t\t\t\tif (bracket[-1] == ' ')\n \t\t\t\t\t\tbracket--;\n \n \t\t\t\t\tauthor = buffer + 7;\n-- \n1.4.4.2.g3528-dirty\n\nOn Fri, Dec 08, 2006 at 10:23:14PM -0500, Nicolas Pitre wrote:\n\n> On the Linux kernel repository, doing \"git shortlog v2.6.18..\" works \n> fine. \"git shortlog v2.6.17..\" works fine. \"git shortlog v2.6.16..\" also \n> works fine. But \"git shortlog v2.6.15..\" dies with a segmentation \n> fault. Trying \"git log v2.6.15.. | git shortlog\" also produces the same \n> crash while \"git log v2.6.16.. | git shortlog\" works fine.\n> \n> The old perl version doesn't have any such issue with those test cases, \n> not even with the whole kernel history.\n> \n> \n> Nicolas\n> -\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n"},{"id":"296621","messageId":"Pine.LNX.4.64.0612091653070.2630@xanadu.home","threadId":"43213","inReplyTo":"20061209040421.GA29113@coredump.intra.peff.net","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","fromName":"Nicolas Pitre","fromEmail":"nico@cam.org","sentAt":"2006-12-09T21:56:08Z","receivedAt":"2006-12-09T21:56:08Z","isPatch":true,"sender":{"key":"nico@fluxnic.net","avatar":"https://avatars.githubusercontent.com/u/702790?v=4"},"body":"On Fri, 8 Dec 2006, Jeff King wrote:\n\n> The old code looked backwards from the email address to parse the name,\n> allowing an arbitrary number of spaces between the two. However, in the case\n> of no name, we looked back too far to the 'author' (or 'Author:') header.\n> Instead, remove at most one space between name and address.\n> \n> The bug was triggered by commit febf7ea4bed from linux-2.6.\n> \n> Signed-off-by: Jeff King \n\nYep, that fixes the problem.\n\n\n"},{"id":"294048","messageId":"Pine.LNX.4.63.0612100019270.28348@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43213","inReplyTo":"20061209040421.GA29113@coredump.intra.peff.net","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-09T23:21:03Z","receivedAt":"2006-12-09T23:21:03Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 Dec 2006, Jeff King wrote:\n\n> Instead, remove at most one space between name and address.\n\nWhy? We can fix it properly: Instead of\n\n> -\t\t\t\twhile (isspace(bob[-1]))\n> +\t\t\t\tif (isspace(bob[-1]))\n\ndo something like\n\n\t\t\t\twhile (bob - 1 != buffer + 7 && \n\t\t\t\t\t\tisspace(bob[-1]))\n\nCiao,\nDscho\n"},{"id":"298169","messageId":"Pine.LNX.4.63.0612100034010.28348@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43213","inReplyTo":"Pine.LNX.4.64.0612082205240.2630@xanadu.home","subject":"Re: builtin git-shortlog still broken","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-09T23:34:37Z","receivedAt":"2006-12-09T23:34:37Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 8 Dec 2006, Nicolas Pitre wrote:\n\n> On the Linux kernel repository, doing \"git shortlog v2.6.18..\" works \n> fine. \"git shortlog v2.6.17..\" works fine. \"git shortlog v2.6.16..\" also \n> works fine. But \"git shortlog v2.6.15..\" dies with a segmentation \n> fault. Trying \"git log v2.6.15.. | git shortlog\" also produces the same \n> crash while \"git log v2.6.16.. | git shortlog\" works fine.\n> \n> The old perl version doesn't have any such issue with those test cases, \n> not even with the whole kernel history.\n\nYeah, sorry. I assumed that there are enough sanity checks for the author \nformat, but evidently that is/was not the case here.\n\nCiao,\nDscho\n"},{"id":"297763","messageId":"20061210034524.GA11819@coredump.intra.peff.net","threadId":"43213","inReplyTo":"Pine.LNX.4.63.0612100019270.28348@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2006-12-10T03:45:24Z","receivedAt":"2006-12-10T03:45:24Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Dec 10, 2006 at 12:21:03AM +0100, Johannes Schindelin wrote:\n\n> > Instead, remove at most one space between name and address.\n> \n> Why? We can fix it properly: Instead of\n> \n> > -\t\t\t\twhile (isspace(bob[-1]))\n> > +\t\t\t\tif (isspace(bob[-1]))\n> \n> do something like\n> \n> \t\t\t\twhile (bob - 1 != buffer + 7 && \n> \t\t\t\t\t\tisspace(bob[-1]))\n\nIt doesn't look like there are ever extra spaces to get soaked up in the\nkernel or git repositories, but if there is a reason to expect\n Full Name \nthen we should probably replace my fix with yours.\n\n"},{"id":"294675","messageId":"7vejr85lxq.fsf@assigned-by-dhcp.cox.net","threadId":"43213","inReplyTo":"20061210034524.GA11819@coredump.intra.peff.net","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-10T06:01:21Z","receivedAt":"2006-12-10T06:01:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> It doesn't look like there are ever extra spaces to get soaked up in the\n> kernel or git repositories, but if there is a reason to expect\n> Full Name \n> then we should probably replace my fix with yours.\n\nAs far as I know, commit-tree removes the extra space if exists,\nso your fix would be fine in practice.\n\nPeople could write crappy replacements of plumbing, and we may\nend up with names with trailing whitespaces, but with your fix\nat least that would not make us crash, and it may even be\nconsidered a feature -- it would serve as a coalmine canary to\ndetect such a broken alternative program that creates bogus\ncommit objects.\n\nWe might want to tighten fsck-objects to warn about them,\nthough. But that is probably rather low priority.\n"},{"id":"294915","messageId":"Pine.LNX.4.63.0612102342490.28348@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43213","inReplyTo":"20061210034524.GA11819@coredump.intra.peff.net","subject":"Re: [PATCH] shortlog: fix segfault on empty authorname","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-10T22:43:19Z","receivedAt":"2006-12-10T22:43:19Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 9 Dec 2006, Jeff King wrote:\n\n> On Sun, Dec 10, 2006 at 12:21:03AM +0100, Johannes Schindelin wrote:\n> \n> > > Instead, remove at most one space between name and address.\n> > \n> > Why? We can fix it properly: Instead of\n> > \n> > > -\t\t\t\twhile (isspace(bob[-1]))\n> > > +\t\t\t\tif (isspace(bob[-1]))\n> > \n> > do something like\n> > \n> > \t\t\t\twhile (bob - 1 != buffer + 7 && \n> > \t\t\t\t\t\tisspace(bob[-1]))\n> \n> It doesn't look like there are ever extra spaces to get soaked up in the\n> kernel or git repositories, but if there is a reason to expect\n> Full Name \n> then we should probably replace my fix with yours.\n\nThis is exactly the kind of assumption which led to the bug to begin with.\n\nCiao,\nDscho\n"}]}