{"thread":{"id":"8610","subject":"[PATCH/RFH] pp_header(): work around possible memory corruption","startedAt":"2007-06-15T12:19:07Z","lastAt":"2007-06-23T23:32:58Z","messageCount":4,"participants":["Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"45131","messageId":"Pine.LNX.4.64.0706151318210.4059@racer.site","threadId":"8610","inReplyTo":null,"subject":"[PATCH/RFH] pp_header(): work around possible memory corruption","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-15T12:19:07Z","receivedAt":"2007-06-15T12:19:07Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nadd_user_info() possibly adds way more than just the commit header line. \nIn fact, it sometimes needs so much more space that there is a buffer \noverrun, leading to an ugly crash. For example, the date is printed in its \nown line, and usually takes up more space than the equivalent Unix epoch.\n\nSo, for good measure, add 80 characters (a full line) to the allocated \nspace, in addition to the header line length.\n\nSigned-off-by: Johannes Schindelin <Johannes.Schindelin@gmx.de>\n\n---\n\n\tI have no idea if 80 is a good value, and if other places\n\tneed an equivalent fix up, too.\n\n\tBut I needed this patch in a hurry...\n\n commit.c |    4 ++--\n 1 files changed, 2 insertions(+), 2 deletions(-)\n\ndiff --git a/commit.c b/commit.c\nindex f778bf4..03436b1 100644\n--- a/commit.c\n+++ b/commit.c\n@@ -997,7 +997,7 @@ static void pp_header(enum cmit_fmt fmt,\n \t\t\tlen = linelen;\n \t\t\tif (fmt == CMIT_FMT_EMAIL)\n \t\t\t\tlen = bound_rfc2047(linelen, encoding);\n-\t\t\tALLOC_GROW(*buf_p, *ofs_p + len, *space_p);\n+\t\t\tALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);\n \t\t\tdst = *buf_p + *ofs_p;\n \t\t\t*ofs_p += add_user_info(\"Author\", fmt, dst,\n \t\t\t\t\t\tline + 7, dmode, encoding);\n@@ -1008,7 +1008,7 @@ static void pp_header(enum cmit_fmt fmt,\n \t\t\tlen = linelen;\n \t\t\tif (fmt == CMIT_FMT_EMAIL)\n \t\t\t\tlen = bound_rfc2047(linelen, encoding);\n-\t\t\tALLOC_GROW(*buf_p, *ofs_p + len, *space_p);\n+\t\t\tALLOC_GROW(*buf_p, *ofs_p + len + 80, *space_p);\n \t\t\tdst = *buf_p + *ofs_p;\n \t\t\t*ofs_p += add_user_info(\"Commit\", fmt, dst,\n \t\t\t\t\t\tline + 10, dmode, encoding);\n"},{"id":"45159","messageId":"7vps3w4g9x.fsf@assigned-by-dhcp.pobox.com","threadId":"8610","inReplyTo":"Pine.LNX.4.64.0706151318210.4059@racer.site","subject":"Re: [PATCH/RFH] pp_header(): work around possible memory corruption","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-06-16T05:31:06Z","receivedAt":"2007-06-16T05:31:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"At least the older humongous pretty_print_commit() got separated\ninto manageable chunks, and I was happy.  I was just too lazy\nwhen refactoring the code and stopped there.\n\nThe right fix is to propagate the \"realloc as needed\" callchain\ninto add_user_info(), instead of having \"this should be enough\"\nthere.  These two you touched are the only two callsite of that\nstatic function.\n"},{"id":"45301","messageId":"Pine.LNX.4.64.0706190118230.4059@racer.site","threadId":"8610","inReplyTo":"7vps3w4g9x.fsf@assigned-by-dhcp.pobox.com","subject":"Re: [PATCH/RFH] pp_header(): work around possible memory corruption","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-19T00:19:37Z","receivedAt":"2007-06-19T00:19:37Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 Jun 2007, Junio C Hamano wrote:\n\n> At least the older humongous pretty_print_commit() got separated into \n> manageable chunks, and I was happy.  I was just too lazy when \n> refactoring the code and stopped there.\n\nThat's perfectly okay. This is why you parked it in 'next', I guess.\n\n> The right fix is to propagate the \"realloc as needed\" callchain into \n> add_user_info(), instead of having \"this should be enough\" there.  \n> These two you touched are the only two callsite of that static function.\n\nRight. As I said, I was in a hurry, and could not research it properly. \nBesides, now that you gave me the proper pointer, I can take care about it \ntomorrow, unless somebody else is faster.\n\nCiao,\nDscho\n"},{"id":"45630","messageId":"Pine.LNX.4.64.0706240031220.4059@racer.site","threadId":"8610","inReplyTo":"7vps3w4g9x.fsf@assigned-by-dhcp.pobox.com","subject":"Re: [PATCH/RFH] pp_header(): work around possible memory corruption","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-06-23T23:32:58Z","receivedAt":"2007-06-23T23:32:58Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Fri, 15 Jun 2007, Junio C Hamano wrote:\n\n> At least the older humongous pretty_print_commit() got separated into \n> manageable chunks, and I was happy.  I was just too lazy when \n> refactoring the code and stopped there.\n\nUnderstandable.\n\n> The right fix is to propagate the \"realloc as needed\" callchain into \n> add_user_info(), instead of having \"this should be enough\" there.  \n> These two you touched are the only two callsite of that static function.\n\nI had a go at this, but unfortunately I do not understand enough of what \nis going on there. For example, add_rfc2047() sometimes quotes some text. \nI have no idea what a conservative estimate of the growth is, so I cannot \ncontinue there.\n\nSorry,\nDscho\n"}]}