{"thread":{"id":"20361","subject":"Please make git-am handle \\r\\n-damaged patches","startedAt":"2009-08-03T19:08:32Z","lastAt":"2009-08-05T23:10:30Z","messageCount":33,"participants":["H. Peter Anvin","Junio C Hamano","Wesley J. Landaker","Sverre Rabbelier","Paolo Bonzini","Alex Riesen","Brandon Casey","Erik Faye-Lund","Nanako Shiraishi","Tony Finch"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"119427","messageId":"4A7735B0.2040703@zytor.com","threadId":"20361","inReplyTo":null,"subject":"Please make git-am handle \\r\\n-damaged patches","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2009-08-03T19:08:32Z","receivedAt":"2009-08-03T19:08:32Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"In a serious case of craniorectal immersion, the Thunderbird developers\nhave started using \\r\\n line endings on saved emails:\n\nhttps://bugzilla.mozilla.org/show_bug.cgi?id=503271\nhttps://bugzilla.mozilla.org/show_bug.cgi?id=507530\n\nIt would be nice if git-am could handle this case automatically.\n\n\t-hpa\n"},{"id":"119430","messageId":"7vocqw8u0x.fsf@alter.siamese.dyndns.org","threadId":"20361","inReplyTo":"4A7735B0.2040703@zytor.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-03T21:30:38Z","receivedAt":"2009-08-03T21:30:38Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"\"H. Peter Anvin\" <hpa@zytor.com> writes:\n\n> In a serious case of craniorectal immersion, the Thunderbird developers\n> have started using \\r\\n line endings on saved emails:\n>\n> https://bugzilla.mozilla.org/show_bug.cgi?id=503271\n> https://bugzilla.mozilla.org/show_bug.cgi?id=507530\n>\n> It would be nice if git-am could handle this case automatically.\n\nPerhaps\n\n    $ dos2unix *.eml | git am\n"},{"id":"119435","messageId":"200908031556.35634.wjl@icecavern.net","threadId":"20361","inReplyTo":"7vocqw8u0x.fsf@alter.siamese.dyndns.org","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Wesley J. Landaker","fromEmail":"wjl@icecavern.net","sentAt":"2009-08-03T21:56:31Z","receivedAt":"2009-08-03T21:56:31Z","isPatch":false,"sender":{"key":"wjl@icecavern.net","avatar":"https://avatars.githubusercontent.com/u/67229?v=4"},"body":"On Monday 03 August 2009 15:30:38 Junio C Hamano wrote:\n> \"H. Peter Anvin\" <hpa@zytor.com> writes:\n> > In a serious case of craniorectal immersion, the Thunderbird developers\n> > have started using \\r\\n line endings on saved emails:\n> >\n> > https://bugzilla.mozilla.org/show_bug.cgi?id=503271\n> > https://bugzilla.mozilla.org/show_bug.cgi?id=507530\n> >\n> > It would be nice if git-am could handle this case automatically.\n>\n> Perhaps\n>\n>     $ dos2unix *.eml | git am\n\nI didn't try it, but would \"git am\" with \"apply.whitespace\" and \n\"core.whitespace\" set in some reasonable manner help? Not \"automatic\", but \nmay help if dos2unix isn't available for some reason.\n"},{"id":"119438","messageId":"4A7760FD.8020705@zytor.com","threadId":"20361","inReplyTo":"7vocqw8u0x.fsf@alter.siamese.dyndns.org","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2009-08-03T22:13:17Z","receivedAt":"2009-08-03T22:13:17Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"On 08/03/2009 02:30 PM, Junio C Hamano wrote:\n> \"H. Peter Anvin\" <hpa@zytor.com> writes:\n> \n>> In a serious case of craniorectal immersion, the Thunderbird developers\n>> have started using \\r\\n line endings on saved emails:\n>>\n>> https://bugzilla.mozilla.org/show_bug.cgi?id=503271\n>> https://bugzilla.mozilla.org/show_bug.cgi?id=507530\n>>\n>> It would be nice if git-am could handle this case automatically.\n> \n> Perhaps\n> \n>     $ dos2unix *.eml | git am\n> \n\nYes, that's what they suggested, too.  Like I need an extra step in my\npatch process.\n\n\t-hpa\n"},{"id":"119441","messageId":"fabb9a1e0908031521q579a9bf0ta834b2c9253e58e9@mail.gmail.com","threadId":"20361","inReplyTo":"4A7760FD.8020705@zytor.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-03T22:21:46Z","receivedAt":"2009-08-03T22:21:46Z","isPatch":false,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Mon, Aug 3, 2009 at 15:13, H. Peter Anvin<hpa@zytor.com> wrote:\n> Yes, that's what they suggested, too.  Like I need an extra step in my\n> patch process.\n\nWrite your own git-lazy-am.sh and put it in your path?\n\ncat > git-lazy-am.sh << EOF\n#!/bin/bash\n\ndos2unix \"$@\" | git am\nEOF\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"119442","messageId":"4A77655E.3010902@zytor.com","threadId":"20361","inReplyTo":"fabb9a1e0908031521q579a9bf0ta834b2c9253e58e9@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2009-08-03T22:31:58Z","receivedAt":"2009-08-03T22:31:58Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"On 08/03/2009 03:21 PM, Sverre Rabbelier wrote:\n> Heya,\n> \n> On Mon, Aug 3, 2009 at 15:13, H. Peter Anvin<hpa@zytor.com> wrote:\n>> Yes, that's what they suggested, too.  Like I need an extra step in my\n>> patch process.\n> \n> Write your own git-lazy-am.sh and put it in your path?\n> \n> cat > git-lazy-am.sh << EOF\n> #!/bin/bash\n> \n> dos2unix \"$@\" | git am\n> EOF\n\nYes, I can do that.  However, having this integrated into git am would\nbe nicer not just for me but for everyone else.\n\n(Currently I'm just using apply.whitespace = fix, which also solves the\nissue.)\n\n\t-hpa\n"},{"id":"119443","messageId":"4A776B32.4070509@zytor.com","threadId":"20361","inReplyTo":"200908031556.35634.wjl@icecavern.net","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"H. Peter Anvin","fromEmail":"hpa@zytor.com","sentAt":"2009-08-03T22:56:50Z","receivedAt":"2009-08-03T22:56:50Z","isPatch":false,"sender":{"key":"hpa@zytor.com","avatar":null},"body":"On 08/03/2009 02:56 PM, Wesley J. Landaker wrote:\n> On Monday 03 August 2009 15:30:38 Junio C Hamano wrote:\n>> \"H. Peter Anvin\" <hpa@zytor.com> writes:\n>>> In a serious case of craniorectal immersion, the Thunderbird developers\n>>> have started using \\r\\n line endings on saved emails:\n>>>\n>>> https://bugzilla.mozilla.org/show_bug.cgi?id=503271\n>>> https://bugzilla.mozilla.org/show_bug.cgi?id=507530\n>>>\n>>> It would be nice if git-am could handle this case automatically.\n>> Perhaps\n>>\n>>     $ dos2unix *.eml | git am\n> \n> I didn't try it, but would \"git am\" with \"apply.whitespace\" and \n> \"core.whitespace\" set in some reasonable manner help? Not \"automatic\", but \n> may help if dos2unix isn't available for some reason.\n\nIt works, but it's extremely noisy.\n\n\t-hpa\n"},{"id":"119463","messageId":"4A77D36C.9060309@gnu.org","threadId":"20361","inReplyTo":"fabb9a1e0908031521q579a9bf0ta834b2c9253e58e9@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Paolo Bonzini","fromEmail":"bonzini@gnu.org","sentAt":"2009-08-04T06:21:32Z","receivedAt":"2009-08-04T06:21:32Z","isPatch":false,"sender":{"key":"bonzini@gnu.org","avatar":"https://avatars.githubusercontent.com/u/42082?v=4"},"body":"On 08/04/2009 12:21 AM, Sverre Rabbelier wrote:\n> Heya,\n>\n> On Mon, Aug 3, 2009 at 15:13, H. Peter Anvin<hpa@zytor.com>  wrote:\n>> Yes, that's what they suggested, too.  Like I need an extra step in my\n>> patch process.\n>\n> Write your own git-lazy-am.sh and put it in your path?\n>\n> cat>  git-lazy-am.sh<<  EOF\n> #!/bin/bash\n>\n> dos2unix \"$@\" | git am\n> EOF\n\nEven better than putting it on the path,\n\n[alias]\n\ttbam = ~/libexec/git/git-lazy-am.sh\n\nPaolo\n"},{"id":"119466","messageId":"81b0412b0908032335s3363849aj32a0e93efa15c012@mail.gmail.com","threadId":"20361","inReplyTo":"4A7735B0.2040703@zytor.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T06:35:24Z","receivedAt":"2009-08-04T06:35:24Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Mon, Aug 3, 2009 at 21:08, H. Peter Anvin<hpa@zytor.com> wrote:\n> In a serious case of craniorectal immersion, the Thunderbird developers\n> have started using \\r\\n line endings on saved emails:\n>\n> https://bugzilla.mozilla.org/show_bug.cgi?id=503271\n> https://bugzilla.mozilla.org/show_bug.cgi?id=507530\n>\n> It would be nice if git-am could handle this case automatically.\n>\n\nMaybe it is as simple as that (not tested yet,\nand sent through gmail, so please be careful):\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex ad5f6b5..02c1c92 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n \t\tif (c == '\\n' || len + 1 >= size)\n \t\t\tbreak;\n \t}\n+\tif (len && buf[len - 1] == '\\r')\n+\t\t--len;\n \tbuf[len] = '\\0';\n\n \treturn len;\n"},{"id":"119469","messageId":"7v7hxk5b4y.fsf@alter.siamese.dyndns.org","threadId":"20361","inReplyTo":"81b0412b0908032335s3363849aj32a0e93efa15c012@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T06:46:53Z","receivedAt":"2009-08-04T06:46:53Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Alex Riesen <raa.lkml@gmail.com> writes:\n\n> Maybe it is as simple as that (not tested yet,\n> and sent through gmail, so please be careful):\n\nI thought about this approach, but it made me worried about a case where\nan otherwise sane piece of e-mail has \\r at the end of one line as the\nreal payload.  But as long as we are talking about a text e-mail (and we\nare talking about mailsplit here and a binary payload with a CTE applied\ncounts as text), I think we can safely use an approach like this.\n\n>\n> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\n> index ad5f6b5..02c1c92 100644\n> --- a/builtin-mailsplit.c\n> +++ b/builtin-mailsplit.c\n> @@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n>  \t\tif (c == '\\n' || len + 1 >= size)\n>  \t\t\tbreak;\n>  \t}\n> +\tif (len && buf[len - 1] == '\\r')\n> +\t\t--len;\n>  \tbuf[len] = '\\0';\n>\n>  \treturn len;\n"},{"id":"119507","messageId":"20090804172638.GA15136@blimp.localdomain","threadId":"20361","inReplyTo":"7v7hxk5b4y.fsf@alter.siamese.dyndns.org","subject":"[PATCH] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T17:26:38Z","receivedAt":"2009-08-04T17:26:38Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"It is not that uncommon to have mails with DOS line-ending, notably\nThunderbird and Gmail (when saving what they call \"original\" message).\n\nNoticed by H. Peter Anvin.\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nAlex Riesen <raa.lkml@gmail.com> writes:\n\n> Maybe it is as simple as that (not tested yet,\n> and sent through gmail, so please be careful):\n\n> --- a/builtin-mailsplit.c\n> +++ b/builtin-mailsplit.c\n> @@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n> +\tif (len && buf[len - 1] == '\\r')\n> +\t\t--len;\n\nThat's wrong, of course. I missed the fact that \\n stays in the\nbuffer. Corrected.\n\n builtin-mailsplit.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex ad5f6b5..188ffec 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n \t\tif (c == '\\n' || len + 1 >= size)\n \t\t\tbreak;\n \t}\n+\tif (len > 1 && buf[len - 2] == '\\r')\n+\t\tbuf[--len - 1] = '\\n';\n \tbuf[len] = '\\0';\n \n \treturn len;\n-- \n1.6.4.30.gd4c0\n"},{"id":"119508","messageId":"fabb9a1e0908041034y1a43fdf6nae14ec106a82f00c@mail.gmail.com","threadId":"20361","inReplyTo":"20090804172638.GA15136@blimp.localdomain","subject":"Re: [PATCH] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Sverre Rabbelier","fromEmail":"srabbelier@gmail.com","sentAt":"2009-08-04T17:34:37Z","receivedAt":"2009-08-04T17:34:37Z","isPatch":true,"sender":{"key":"srabbelier@gmail.com","avatar":"https://avatars.githubusercontent.com/u/3098?v=4"},"body":"Heya,\n\nOn Tue, Aug 4, 2009 at 10:26, Alex Riesen<raa.lkml@gmail.com> wrote:\n>        }\n> +       if (len > 1 && buf[len - 2] == '\\r')\n> +               buf[--len - 1] = '\\n';\n>        buf[len] = '\\0';\n\nHow about something like:\n\n+       if (len > 1 && buf[len - 2] == '\\r' && (buf[len - 1] == '\\n'\n|| buf[len - 1] == '\\0'))\n+               buf[--len - 1] = '\\n';\n\nTo make sure that we're not erasing a \\r somewhere in the middle of the content?\n\n-- \nCheers,\n\nSverre Rabbelier\n"},{"id":"119510","messageId":"pUtiIFYWaIGcw7mPKZiIk4XSCpgEimhFB1QSpwG5P0mtMWR_ZefDdg@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"fabb9a1e0908041034y1a43fdf6nae14ec106a82f00c@mail.gmail.com","subject":"Re: [PATCH] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-08-04T18:50:28Z","receivedAt":"2009-08-04T18:50:28Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Sverre Rabbelier wrote:\n> Heya,\n> \n> On Tue, Aug 4, 2009 at 10:26, Alex Riesen<raa.lkml@gmail.com> wrote:\n>>        }\n>> +       if (len > 1 && buf[len - 2] == '\\r')\n>> +               buf[--len - 1] = '\\n';\n>>        buf[len] = '\\0';\n> \n> How about something like:\n> \n> +       if (len > 1 && buf[len - 2] == '\\r' && (buf[len - 1] == '\\n'\n> || buf[len - 1] == '\\0'))\n> +               buf[--len - 1] = '\\n';\n> \n> To make sure that we're not erasing a \\r somewhere in the middle of the content?\n\nYou may want to push the \\r back into the buffer if it is the last character read\ntoo.  We may reach the limit of size characters without finding a \\n, and so we\ncan't tell whether the last \\r we read was a solitary \\r or whether it is the\nbeginning of \\r\\n sequence.\n\nSo maybe we need something like this after the 'for' loop instead:\n\n\n   if (c == '\\n') {\n   \tif (len > 1 && buf[len - 2] == '\\r')\n\t\tbuf[--len - 1] = '\\n';\n   } else if (c == '\\r') {\n\tungetc(c, in);\n\tlen--;\n   }\n\n-brandon\n"},{"id":"119517","messageId":"81b0412b0908041357he6056a3l78ebcec413a2e255@mail.gmail.com","threadId":"20361","inReplyTo":"fabb9a1e0908041034y1a43fdf6nae14ec106a82f00c@mail.gmail.com","subject":"Re: [PATCH] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T20:57:36Z","receivedAt":"2009-08-04T20:57:36Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Aug 4, 2009 at 19:34, Sverre Rabbelier<srabbelier@gmail.com> wrote:\n> On Tue, Aug 4, 2009 at 10:26, Alex Riesen<raa.lkml@gmail.com> wrote:\n>>        }\n>> +       if (len > 1 && buf[len - 2] == '\\r')\n>> +               buf[--len - 1] = '\\n';\n>>        buf[len] = '\\0';\n>\n> How about something like:\n>\n> +       if (len > 1 && buf[len - 2] == '\\r' && (buf[len - 1] == '\\n'\n> || buf[len - 1] == '\\0'))\n> +               buf[--len - 1] = '\\n';\n>\n> To make sure that we're not erasing a \\r somewhere in the middle of the content?\n\nI think this should be enough:\n\n\tif (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n\t\tbuf[--len - 1] = '\\n';\n\nI'll resend.\n"},{"id":"119518","messageId":"40aa078e0908041359p79b111eo2f389dde25bb0876@mail.gmail.com","threadId":"20361","inReplyTo":"7v7hxk5b4y.fsf@alter.siamese.dyndns.org","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-08-04T20:59:47Z","receivedAt":"2009-08-04T20:59:47Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Aug 4, 2009 at 8:46 AM, Junio C Hamano<gitster@pobox.com> wrote:\n> I thought about this approach, but it made me worried about a case where\n> an otherwise sane piece of e-mail has \\r at the end of one line as the\n> real payload. But as long as we are talking about a text e-mail (and we\n> are talking about mailsplit here and a binary payload with a CTE applied\n> counts as text), I think we can safely use an approach like this.\n\nRFC 2822, section 2.3 explicitly states that a CR should not occur\nwithout a LF (and vice versa, but the e-mail client might convert CRLF\nto LF when saving to file), so I think this should be safe.\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"119519","messageId":"20090804210327.GA23747@blimp.localdomain","threadId":"20361","inReplyTo":"20090804172638.GA15136@blimp.localdomain","subject":"[PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T21:03:27Z","receivedAt":"2009-08-04T21:03:27Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"Noticed by H. Peter Anvin.\n\nIt is not that uncommon to have mails with DOS line-ending,\nnotably Thunderbird and web mailers like Gmail (when saving\nwhat they call \"original\" message).\n\nSigned-off-by: Alex Riesen <raa.lkml@gmail.com>\n---\n\nCorrected bug with unconditonal last (or very long) line shortening if\nit contains a CR in next-to-last character. Noticed by Sverre Rabbelier.\n\nIt should also handle the case mentioned by Brandon Casey.\n\n builtin-mailsplit.c |    2 ++\n 1 files changed, 2 insertions(+), 0 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex ad5f6b5..48285a0 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n \t\tif (c == '\\n' || len + 1 >= size)\n \t\t\tbreak;\n \t}\n+\tif (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n+\t\tbuf[--len - 1] = '\\n';\n \tbuf[len] = '\\0';\n \n \treturn len;\n-- \n1.6.4.34.gc3135e\n"},{"id":"119520","messageId":"81b0412b0908041405u1da859f2lf23d6ba25e3bef1f@mail.gmail.com","threadId":"20361","inReplyTo":"40aa078e0908041359p79b111eo2f389dde25bb0876@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T21:05:54Z","receivedAt":"2009-08-04T21:05:54Z","isPatch":false,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Aug 4, 2009 at 22:59, Erik Faye-Lund<kusmabite@googlemail.com> wrote:\n> On Tue, Aug 4, 2009 at 8:46 AM, Junio C Hamano<gitster@pobox.com> wrote:\n>> I thought about this approach, but it made me worried about a case where\n>> an otherwise sane piece of e-mail has \\r at the end of one line as the\n>> real payload. But as long as we are talking about a text e-mail (and we\n>> are talking about mailsplit here and a binary payload with a CTE applied\n>> counts as text), I think we can safely use an approach like this.\n>\n> RFC 2822, section 2.3 explicitly states that a CR should not occur\n> without a LF (and vice versa, but the e-mail client might convert CRLF\n> to LF when saving to file), so I think this should be safe.\n\nYou missed this line in original posters e-mail:\n\"In a serious case of craniorectal immersion...\"\n\nWe are not safe from that and alike.\n"},{"id":"119521","messageId":"20090805061514.6117@nanako3.lavabit.com","threadId":"20361","inReplyTo":"7v7hxk5b4y.fsf@alter.siamese.dyndns.org","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Nanako Shiraishi","fromEmail":"nanako3@lavabit.com","sentAt":"2009-08-04T21:15:14Z","receivedAt":"2009-08-04T21:15:14Z","isPatch":false,"sender":{"key":"nanako3@lavabit.com","avatar":"https://gravatar.com/avatar/3777b9e201c5883a62b1a6fdf7c53f2d712d1d80989146063ea861e33aad72a8?d=mp&s=160"},"body":"Quoting Junio C Hamano <gitster@pobox.com>\n\n> Alex Riesen <raa.lkml@gmail.com> writes:\n>\n>> Maybe it is as simple as that (not tested yet,\n>> and sent through gmail, so please be careful):\n>\n> I thought about this approach, but it made me worried about a case where\n> an otherwise sane piece of e-mail has \\r at the end of one line as the\n> real payload.  But as long as we are talking about a text e-mail (and we\n> are talking about mailsplit here and a binary payload with a CTE applied\n> counts as text), I think we can safely use an approach like this.\n\nIs it safe to rebase a commit that introduces a carriage-return at the end of the line using the updated program?\n\n-- \nNanako Shiraishi\nhttp://ivory.ap.teacup.com/nanako3/\n"},{"id":"119524","messageId":"40aa078e0908041416t69ad9d8dxda218489f675f82e@mail.gmail.com","threadId":"20361","inReplyTo":"81b0412b0908041405u1da859f2lf23d6ba25e3bef1f@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Erik Faye-Lund","fromEmail":"kusmabite@googlemail.com","sentAt":"2009-08-04T21:16:06Z","receivedAt":"2009-08-04T21:16:06Z","isPatch":false,"sender":{"key":"kusmabite@gmail.com","avatar":"https://avatars.githubusercontent.com/u/47073?v=4"},"body":"On Tue, Aug 4, 2009 at 11:05 PM, Alex Riesen<raa.lkml@gmail.com> wrote:\n> You missed this line in original posters e-mail:\n> \"In a serious case of craniorectal immersion...\"\n>\n> We are not safe from that and alike.\n\nStoring e-mails with CRLFs intact is perfectly sane. Depending on them\nnot being stored that way is not.\n\n-- \nErik \"kusma\" Faye-Lund\nkusmabite@gmail.com\n(+47) 986 59 656\n"},{"id":"119526","messageId":"H9KDJzU0XgndJHC8J4OETn7kxYhGP3gl0YJPR0low0hfIHjZOUsoxw@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"20090804210327.GA23747@blimp.localdomain","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-08-04T21:29:54Z","receivedAt":"2009-08-04T21:29:54Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Alex Riesen wrote:\n> Noticed by H. Peter Anvin.\n> \n> It is not that uncommon to have mails with DOS line-ending,\n> notably Thunderbird and web mailers like Gmail (when saving\n> what they call \"original\" message).\n> \n> Signed-off-by: Alex Riesen <raa.lkml@gmail.com>\n> ---\n> \n> Corrected bug with unconditonal last (or very long) line shortening if\n> it contains a CR in next-to-last character. Noticed by Sverre Rabbelier.\n> \n> It should also handle the case mentioned by Brandon Casey.\n> \n>  builtin-mailsplit.c |    2 ++\n>  1 files changed, 2 insertions(+), 0 deletions(-)\n> \n> diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\n> index ad5f6b5..48285a0 100644\n> --- a/builtin-mailsplit.c\n> +++ b/builtin-mailsplit.c\n> @@ -58,6 +58,8 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n>  \t\tif (c == '\\n' || len + 1 >= size)\n>  \t\t\tbreak;\n>  \t}\n> +\tif (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n> +\t\tbuf[--len - 1] = '\\n';\n>  \tbuf[len] = '\\0';\n>  \n>  \treturn len;\n\nWhat if \\r lands at character 99 and \\n is at character 100?  If buf has\nexactly 100 characters available for writing.  Wouldn't the '\\r' be stored\ninto buf, but not the '\\n'?  Then the 'for' loop would terminate since len + 1\nwould be >= size and the code above would test whether c == '\\n', and it\nwould not, so the '\\r' would not be removed from buf as it should be.\n\nAt the point where buf has been filled, and the last character read is a '\\r',\nwe can not tell whether the next character is a '\\n' or not, so we do not know\nif it is a solitary '\\r' or whether it is the start of a '\\r\\n' sequence.  It\nseems to me that we must push the '\\r' back into the stream and allow the next\ncall to read_line_with_nul() handle it, or peek to see if the next character\nis a '\\n'.\n\nBe aware that if we use the version I suggested which pushes the '\\r' back\ninto the input stream, I think we risk an infinite loop if size == 1.  I don't\nthink that is possible from the current callers though.\n\n-brandon\n"},{"id":"119528","messageId":"-WLgEY11_FJV7byR76pdKYY69wMV5WIIRr-HvzsgumvNMS0FNKTSfA@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"H9KDJzU0XgndJHC8J4OETn7kxYhGP3gl0YJPR0low0hfIHjZOUsoxw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-08-04T21:40:57Z","receivedAt":"2009-08-04T21:40:57Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Brandon Casey wrote:\n\n> Be aware that if we use the version I suggested which pushes the '\\r' back\n> into the input stream, I think we risk an infinite loop if size == 1.  I don't\n> think that is possible from the current callers though.\n\nI think something like this would avoid any potential infinite loop, however improbable:\n\n        if (c == '\\n') {\n                if (len > 1 && buf[len - 2] == '\\r')\n                        buf[--len - 1] = '\\n';\n        } else if (c == '\\r') {\n                c = getc(in);\n                if (c == '\\n')\n                        buf[len - 1] = '\\n';\n                else if (c != EOF)\n                        ungetc(c, in);\n        }\n\n-brandon\n"},{"id":"119531","messageId":"81b0412b0908041458w7e76854bn96c4cb71a5e91e74@mail.gmail.com","threadId":"20361","inReplyTo":"H9KDJzU0XgndJHC8J4OETn7kxYhGP3gl0YJPR0low0hfIHjZOUsoxw@cipher.nrlssc.navy.mil","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T21:58:13Z","receivedAt":"2009-08-04T21:58:13Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Aug 4, 2009 at 23:29, Brandon\nCasey<brandon.casey.ctr@nrlssc.navy.mil> wrote:\n> Alex Riesen wrote:\n>> +     if (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n>> +             buf[--len - 1] = '\\n';\n>>       buf[len] = '\\0';\n>>\n>>       return len;\n>\n> What if \\r lands at character 99 and \\n is at character 100?  If buf has\n> exactly 100 characters available for writing. ...\n\nAh, yes. You're right.\n\nI have strong dislike towards unget, though. How about this, instead:\n\nint read_line_with_nul(char *buf, int size, FILE *in)\n{\n\tint len = 0, c;\n\n\twhile (len < size) {\n\t\tc = getc(in);\n\t\tif (c == EOF)\n\t\t\tbreak;\n\t\tbuf[len++] = c;\n\t\tif (c == '\\n')\n\t\t\tbreak;\n\t\telse if (len == size)\n\t\t\tc = 0;\n\t}\n\tif (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n\t\tbuf[--len - 1] = '\\n';\n\tbuf[len] = '\\0';\n\n\treturn len;\n}\n"},{"id":"119532","messageId":"81b0412b0908041503o21f6347ewe60e92a4cb277fd9@mail.gmail.com","threadId":"20361","inReplyTo":"81b0412b0908041458w7e76854bn96c4cb71a5e91e74@mail.gmail.com","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T22:03:08Z","receivedAt":"2009-08-04T22:03:08Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Tue, Aug 4, 2009 at 23:58, Alex Riesen<raa.lkml@gmail.com> wrote:\n> int read_line_with_nul(char *buf, int size, FILE *in)\n> {\n>        int len = 0, c;\n>\n>        while (len < size) {\n\n\"for (;;)\", of course. Leftover from development process\n\n>                c = getc(in);\n>                if (c == EOF)\n>                        break;\n>                buf[len++] = c;\n>                if (c == '\\n')\n>                        break;\n>                else if (len == size)\n>                        c = 0;\n>        }\n>        if (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n>                buf[--len - 1] = '\\n';\n>        buf[len] = '\\0';\n>\n>        return len;\n> }\n>\n"},{"id":"119533","messageId":"81b0412b0908041504p335c05c9ibed039a4b3cd9c33@mail.gmail.com","threadId":"20361","inReplyTo":"81b0412b0908041503o21f6347ewe60e92a4cb277fd9@mail.gmail.com","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-04T22:04:13Z","receivedAt":"2009-08-04T22:04:13Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Wed, Aug 5, 2009 at 00:03, Alex Riesen<raa.lkml@gmail.com> wrote:\n> On Tue, Aug 4, 2009 at 23:58, Alex Riesen<raa.lkml@gmail.com> wrote:\n>> int read_line_with_nul(char *buf, int size, FILE *in)\n>> {\n>>        int len = 0, c;\n>>\n>>        while (len < size) {\n>\n> \"for (;;)\", of course. Leftover from development process\n\nAhm... No, its not! Its correct :)\nGood night, everybody.\n\n>>                c = getc(in);\n>>                if (c == EOF)\n>>                        break;\n>>                buf[len++] = c;\n>>                if (c == '\\n')\n>>                        break;\n>>                else if (len == size)\n>>                        c = 0;\n>>        }\n>>        if (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n>>                buf[--len - 1] = '\\n';\n>>        buf[len] = '\\0';\n>>\n>>        return len;\n>> }\n>>\n>\n"},{"id":"119536","messageId":"7vmy6fdxst.fsf@alter.siamese.dyndns.org","threadId":"20361","inReplyTo":"20090805061514.6117@nanako3.lavabit.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2009-08-04T22:22:26Z","receivedAt":"2009-08-04T22:22:26Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Nanako Shiraishi <nanako3@lavabit.com> writes:\n\n> Quoting Junio C Hamano <gitster@pobox.com>\n>\n>> Alex Riesen <raa.lkml@gmail.com> writes:\n>>\n>>> Maybe it is as simple as that (not tested yet,\n>>> and sent through gmail, so please be careful):\n>>\n>> I thought about this approach, but it made me worried about a case where\n>> an otherwise sane piece of e-mail has \\r at the end of one line as the\n>> real payload.  But as long as we are talking about a text e-mail (and we\n>> are talking about mailsplit here and a binary payload with a CTE applied\n>> counts as text), I think we can safely use an approach like this.\n>\n> Is it safe to rebase a commit that introduces a carriage-return at the end of the line using the updated program?\n\nHmmm, good point.  The approach does break rebase.\n\nAt least we could patch it up like this.\n\n builtin-mailinfo.c  |    2 +-\n builtin-mailsplit.c |   11 +++++++++--\n builtin.h           |    2 +-\n git-am.sh           |    8 +++++++-\n t/t3400-rebase.sh   |   26 ++++++++++++++++++++++++--\n 5 files changed, 42 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 92637ac..6e44d2c 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -835,7 +835,7 @@ static void handle_body(void)\n \t\tstrbuf_reset(&line);\n \t\tif (strbuf_avail(&line) < 100)\n \t\t\tstrbuf_grow(&line, 100);\n-\t} while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin)));\n+\t} while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin, 0)));\n \n handle_body_out:\n \tstrbuf_release(&prev);\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex ad5f6b5..f774b04 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -45,8 +45,10 @@ static int is_from_line(const char *line, int len)\n /* Could be as small as 64, enough to hold a Unix \"From \" line. */\n static char buf[4096];\n \n+static int keep_cr;\n+\n /* We cannot use fgets() because our lines can contain NULs */\n-int read_line_with_nul(char *buf, int size, FILE *in)\n+int read_line_with_nul(char *buf, int size, FILE *in, int nuke_cr_at_eol)\n {\n \tint len = 0, c;\n \n@@ -58,6 +60,9 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n \t\tif (c == '\\n' || len + 1 >= size)\n \t\t\tbreak;\n \t}\n+\tif (nuke_cr_at_eol &&\n+\t    (1 < len && buf[len - 2] == '\\r' && buf[len - 1] == '\\n'))\n+\t\tbuf[len-- - 2] = '\\n';\n \tbuf[len] = '\\0';\n \n \treturn len;\n@@ -93,7 +98,7 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n \t\tif (fwrite(buf, 1, len, output) != len)\n \t\t\tdie_errno(\"cannot write output\");\n \n-\t\tlen = read_line_with_nul(buf, sizeof(buf), mbox);\n+\t\tlen = read_line_with_nul(buf, sizeof(buf), mbox, !keep_cr);\n \t\tif (len == 0) {\n \t\t\tif (feof(mbox)) {\n \t\t\t\tstatus = 1;\n@@ -248,6 +253,8 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)\n \t\t\tnr = strtol(arg+2, NULL, 10);\n \t\t} else if ( arg[1] == 'b' && !arg[2] ) {\n \t\t\tallow_bare = 1;\n+\t\t} else if (!strcmp(arg, \"--keep-cr\")) {\n+\t\t\tkeep_cr = 1;\n \t\t} else if ( arg[1] == 'o' && arg[2] ) {\n \t\t\tdir = arg+2;\n \t\t} else if ( arg[1] == '-' && !arg[2] ) {\ndiff --git a/builtin.h b/builtin.h\nindex 825a96f..59dddee 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -13,7 +13,7 @@ extern const char git_more_info_string[];\n extern void list_common_cmds_help(void);\n extern const char *help_unknown_cmd(const char *cmd);\n extern void prune_packed_objects(int);\n-extern int read_line_with_nul(char *buf, int size, FILE *file);\n+extern int read_line_with_nul(char *buf, int size, FILE *file, int nuke_cr_at_eol);\n extern int fmt_merge_msg(int merge_summary, struct strbuf *in,\n \tstruct strbuf *out);\n extern int commit_tree(const char *msg, unsigned char *tree,\ndiff --git a/git-am.sh b/git-am.sh\nindex d64d997..985226b 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -197,7 +197,13 @@ check_patch_format () {\n split_patches () {\n \tcase \"$patch_format\" in\n \tmbox)\n-\t\tgit mailsplit -d\"$prec\" -o\"$dotest\" -b -- \"$@\" > \"$dotest/last\" ||\n+\t\tcase \"$rebasing\" in\n+\t\t'')\n+\t\t\tkeep_cr= ;;\n+\t\t?*)\n+\t\t\tkeep_cr=--keep-cr ;;\n+\t\tesac\n+\t\tgit mailsplit -d\"$prec\" -o\"$dotest\" -b $keep_cr -- \"$@\" > \"$dotest/last\" ||\n \t\tclean_abort\n \t\t;;\n \tstgit-series)\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex c5c29cc..4e6a44b 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -3,9 +3,10 @@\n # Copyright (c) 2005 Amos Waterland\n #\n \n-test_description='git rebase should not destroy author information\n+test_description='git rebase assorted tests\n \n-This test runs git rebase and checks that the author information is not lost.\n+This test runs git rebase and checks that the author information is not lost\n+among other things.\n '\n . ./test-lib.sh\n \n@@ -133,4 +134,25 @@ test_expect_success 'rebase -q is quiet' '\n      test ! -s output.out\n '\n \n+q_to_cr () {\n+\ttr Q '\\015'\n+}\n+\n+test_expect_success 'Rebase a commit that sprinkles CRs in' '\n+\t(\n+\t\techo \"One\"\n+\t\techo \"TwoQ\"\n+\t\techo \"Three\"\n+\t\techo \"FQur\"\n+\t\techo \"Five\"\n+\t) | q_to_cr >CR &&\n+\tgit add CR &&\n+\ttest_tick &&\n+\tgit commit -a -m \"A file with a line with CR\" &&\n+\tgit tag file-with-cr &&\n+\tgit checkout HEAD^0 &&\n+\tgit rebase --onto HEAD^^ HEAD^ &&\n+\tgit diff --exit-code file-with-cr:CR HEAD:CR\n+'\n+\n test_done\n"},{"id":"119538","messageId":"wRnQVVBBRdMB02QMyBKhevKFGLitcB-E36esB_Ar_XFs9upXB_szkQ@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"81b0412b0908041458w7e76854bn96c4cb71a5e91e74@mail.gmail.com","subject":"Re: [PATCH v2] Let mailsplit and mailinfo handle mails with CRLF line-endings","fromName":"Brandon Casey","fromEmail":"brandon.casey.ctr@nrlssc.navy.mil","sentAt":"2009-08-04T22:23:37Z","receivedAt":"2009-08-04T22:23:37Z","isPatch":true,"sender":{"key":"brandon.casey.ctr@nrlssc.navy.mil","avatar":null},"body":"Alex Riesen wrote:\n> On Tue, Aug 4, 2009 at 23:29, Brandon\n> Casey<brandon.casey.ctr@nrlssc.navy.mil> wrote:\n>> Alex Riesen wrote:\n>>> +     if (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n>>> +             buf[--len - 1] = '\\n';\n>>>       buf[len] = '\\0';\n>>>\n>>>       return len;\n>> What if \\r lands at character 99 and \\n is at character 100?  If buf has\n>> exactly 100 characters available for writing. ...\n> \n> Ah, yes. You're right.\n> \n> I have strong dislike towards unget, though. How about this, instead:\n> \n> int read_line_with_nul(char *buf, int size, FILE *in)\n> {\n> \tint len = 0, c;\n> \n> \twhile (len < size) {\n> \t\tc = getc(in);\n> \t\tif (c == EOF)\n> \t\t\tbreak;\n> \t\tbuf[len++] = c;\n> \t\tif (c == '\\n')\n> \t\t\tbreak;\n> \t\telse if (len == size)\n> \t\t\tc = 0;\n> \t}\n> \tif (c == '\\n' && len > 1 && buf[len - 2] == '\\r')\n> \t\tbuf[--len - 1] = '\\n';\n> \tbuf[len] = '\\0';\n> \n> \treturn len;\n> }\n\n\nI don't see how this solves the problem.  Still if the buffer is filled,\nand the last character read is '\\r', and the next character that has not\nyet been read is '\\n', then the '\\r' will erroneously be returned in buf.\n\nPlus, I think you'll have a problem at buf[len] = '\\0' if the loop runs to\ncompletion and len == size.\n\n-brandon\n"},{"id":"119549","messageId":"r3l_p2g-BpVHWKE-kMWIRzBGUCnzo9_l7hOHzYLG_4X6oEjXrJ4rJdB10yXPT2jmJJ7ppBmr-x8@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"7vmy6fdxst.fsf@alter.siamese.dyndns.org","subject":"[PATCH 0/4] Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-05T03:31:55Z","receivedAt":"2009-08-05T03:31:55Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nConvert mailinfo and mailsplit to use strbufs before attempting to address\nthe CRLF issue for saved emails.\n\nBrandon Casey (3):\n  strbuf: add new function strbuf_getwholeline()\n  builtin-mailinfo,builtin-mailsplit: use strbufs\n  builtin-mailsplit.c: remove read_line_with_nul() since it is no\n    longer used\n\nJunio C Hamano (1):\n  Allow mailsplit to handle mails with CRLF line-endings\n\n builtin-mailinfo.c  |    8 +-------\n builtin-mailsplit.c |   44 ++++++++++++++------------------------------\n builtin.h           |    1 -\n git-am.sh           |    8 +++++++-\n strbuf.c            |   15 ++++++++++++---\n strbuf.h            |    1 +\n t/t3400-rebase.sh   |   26 ++++++++++++++++++++++++--\n 7 files changed, 59 insertions(+), 44 deletions(-)\n"},{"id":"119550","messageId":"r3l_p2g-BpVHWKE-kMWIRydJaW0FHLKBpE497REXzOgqPjLUFjPkJ-YKp1tkrIs3CwcppURiH8o@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"r3l_p2g-BpVHWKE-kMWIRzBGUCnzo9_l7hOHzYLG_4X6oEjXrJ4rJdB10yXPT2jmJJ7ppBmr-x8@cipher.nrlssc.navy.mil","subject":"[PATCH 1/4] strbuf: add new function strbuf_getwholeline()","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-05T03:31:56Z","receivedAt":"2009-08-05T03:31:56Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThis function is just like strbuf_getline() except it retains the\nline-termination character.  This function will be used by the mailinfo\nand mailsplit builtins which require the entire line for parsing.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n strbuf.c |   15 ++++++++++++---\n strbuf.h |    1 +\n 2 files changed, 13 insertions(+), 3 deletions(-)\n\ndiff --git a/strbuf.c b/strbuf.c\nindex f03d117..a6153dc 100644\n--- a/strbuf.c\n+++ b/strbuf.c\n@@ -322,7 +322,7 @@ int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint)\n \treturn -1;\n }\n \n-int strbuf_getline(struct strbuf *sb, FILE *fp, int term)\n+int strbuf_getwholeline(struct strbuf *sb, FILE *fp, int term)\n {\n \tint ch;\n \n@@ -332,10 +332,10 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)\n \n \tstrbuf_reset(sb);\n \twhile ((ch = fgetc(fp)) != EOF) {\n-\t\tif (ch == term)\n-\t\t\tbreak;\n \t\tstrbuf_grow(sb, 1);\n \t\tsb->buf[sb->len++] = ch;\n+\t\tif (ch == term)\n+\t\t\tbreak;\n \t}\n \tif (ch == EOF && sb->len == 0)\n \t\treturn EOF;\n@@ -344,6 +344,15 @@ int strbuf_getline(struct strbuf *sb, FILE *fp, int term)\n \treturn 0;\n }\n \n+int strbuf_getline(struct strbuf *sb, FILE *fp, int term)\n+{\n+\tif (strbuf_getwholeline(sb, fp, term))\n+\t\treturn EOF;\n+\tif (sb->buf[sb->len-1] == term)\n+\t\tstrbuf_setlen(sb, sb->len-1);\n+\treturn 0;\n+}\n+\n int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint)\n {\n \tint fd, len;\ndiff --git a/strbuf.h b/strbuf.h\nindex eaa8704..d05e056 100644\n--- a/strbuf.h\n+++ b/strbuf.h\n@@ -126,6 +126,7 @@ extern ssize_t strbuf_read(struct strbuf *, int fd, size_t hint);\n extern int strbuf_read_file(struct strbuf *sb, const char *path, size_t hint);\n extern int strbuf_readlink(struct strbuf *sb, const char *path, size_t hint);\n \n+extern int strbuf_getwholeline(struct strbuf *, FILE *, int);\n extern int strbuf_getline(struct strbuf *, FILE *, int);\n \n extern void stripspace(struct strbuf *buf, int skip_comments);\n-- \n1.6.4\n"},{"id":"119552","messageId":"r3l_p2g-BpVHWKE-kMWIR9UKBn9dqjBL2asOE11gruEtEyCyQOOh37zkh5F2bJkihtkN8WUp4d0@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"r3l_p2g-BpVHWKE-kMWIRydJaW0FHLKBpE497REXzOgqPjLUFjPkJ-YKp1tkrIs3CwcppURiH8o@cipher.nrlssc.navy.mil","subject":"[PATCH 2/4] builtin-mailinfo,builtin-mailsplit: use strbufs","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-05T03:31:57Z","receivedAt":"2009-08-05T03:31:57Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\nThere should be no functional change.  Just the necessary changes and\nsimplifications associated with calling strbuf_getwholeline() rather\nthan an internal function or fgets.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n builtin-mailinfo.c  |    8 +-------\n builtin-mailsplit.c |   20 ++++++++------------\n 2 files changed, 9 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin-mailinfo.c b/builtin-mailinfo.c\nindex 92637ac..b0b5d8f 100644\n--- a/builtin-mailinfo.c\n+++ b/builtin-mailinfo.c\n@@ -765,7 +765,6 @@ static void handle_filter(struct strbuf *line)\n \n static void handle_body(void)\n {\n-\tint len = 0;\n \tstruct strbuf prev = STRBUF_INIT;\n \n \t/* Skip up to the first boundary */\n@@ -775,8 +774,6 @@ static void handle_body(void)\n \t}\n \n \tdo {\n-\t\tstrbuf_setlen(&line, line.len + len);\n-\n \t\t/* process any boundary lines */\n \t\tif (*content_top && is_multipart_boundary(&line)) {\n \t\t\t/* flush any leftover */\n@@ -832,10 +829,7 @@ static void handle_body(void)\n \t\t\thandle_filter(&line);\n \t\t}\n \n-\t\tstrbuf_reset(&line);\n-\t\tif (strbuf_avail(&line) < 100)\n-\t\t\tstrbuf_grow(&line, 100);\n-\t} while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin)));\n+\t} while (!strbuf_getwholeline(&line, fin, '\\n'));\n \n handle_body_out:\n \tstrbuf_release(&prev);\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex ad5f6b5..c2ec6ea 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -7,6 +7,7 @@\n #include \"cache.h\"\n #include \"builtin.h\"\n #include \"string-list.h\"\n+#include \"strbuf.h\"\n \n static const char git_mailsplit_usage[] =\n \"git mailsplit [-d<prec>] [-f<n>] [-b] -o<directory> [<mbox>|<Maildir>...]\";\n@@ -42,8 +43,7 @@ static int is_from_line(const char *line, int len)\n \treturn 1;\n }\n \n-/* Could be as small as 64, enough to hold a Unix \"From \" line. */\n-static char buf[4096];\n+static struct strbuf buf = STRBUF_INIT;\n \n /* We cannot use fgets() because our lines can contain NULs */\n int read_line_with_nul(char *buf, int size, FILE *in)\n@@ -71,10 +71,9 @@ int read_line_with_nul(char *buf, int size, FILE *in)\n static int split_one(FILE *mbox, const char *name, int allow_bare)\n {\n \tFILE *output = NULL;\n-\tint len = strlen(buf);\n \tint fd;\n \tint status = 0;\n-\tint is_bare = !is_from_line(buf, len);\n+\tint is_bare = !is_from_line(buf.buf, buf.len);\n \n \tif (is_bare && !allow_bare)\n \t\tgoto corrupt;\n@@ -88,20 +87,17 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n \t * \"From \" and having something that looks like a date format.\n \t */\n \tfor (;;) {\n-\t\tint is_partial = len && buf[len-1] != '\\n';\n-\n-\t\tif (fwrite(buf, 1, len, output) != len)\n+\t\tif (fwrite(buf.buf, 1, buf.len, output) != buf.len)\n \t\t\tdie_errno(\"cannot write output\");\n \n-\t\tlen = read_line_with_nul(buf, sizeof(buf), mbox);\n-\t\tif (len == 0) {\n+\t\tif (strbuf_getwholeline(&buf, mbox, '\\n')) {\n \t\t\tif (feof(mbox)) {\n \t\t\t\tstatus = 1;\n \t\t\t\tbreak;\n \t\t\t}\n \t\t\tdie_errno(\"cannot read mbox\");\n \t\t}\n-\t\tif (!is_partial && !is_bare && is_from_line(buf, len))\n+\t\tif (!is_bare && is_from_line(buf.buf, buf.len))\n \t\t\tbreak; /* done with one message */\n \t}\n \tfclose(output);\n@@ -166,7 +162,7 @@ static int split_maildir(const char *maildir, const char *dir,\n \t\t\tgoto out;\n \t\t}\n \n-\t\tif (fgets(buf, sizeof(buf), f) == NULL) {\n+\t\tif (strbuf_getwholeline(&buf, f, '\\n')) {\n \t\t\terror(\"cannot read mail %s (%s)\", file, strerror(errno));\n \t\t\tgoto out;\n \t\t}\n@@ -203,7 +199,7 @@ static int split_mbox(const char *file, const char *dir, int allow_bare,\n \t} while (isspace(peek));\n \tungetc(peek, f);\n \n-\tif (fgets(buf, sizeof(buf), f) == NULL) {\n+\tif (strbuf_getwholeline(&buf, f, '\\n')) {\n \t\t/* empty stdin is OK */\n \t\tif (f != stdin) {\n \t\t\terror(\"cannot read mbox %s\", file);\n-- \n1.6.4\n"},{"id":"119553","messageId":"r3l_p2g-BpVHWKE-kMWIR-hbNfTwGmUrXhM-lhoS9dZKPidslwcauSWyJW7X27JpF_HWaZz1tHI@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"r3l_p2g-BpVHWKE-kMWIR9UKBn9dqjBL2asOE11gruEtEyCyQOOh37zkh5F2bJkihtkN8WUp4d0@cipher.nrlssc.navy.mil","subject":"[PATCH 3/4] builtin-mailsplit.c: remove read_line_with_nul() since it is no longer used","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-05T03:31:58Z","receivedAt":"2009-08-05T03:31:58Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Brandon Casey <drafnel@gmail.com>\n\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n\n\nSquash this with the previous one if you like.\n\n-brandon\n\n\n builtin-mailsplit.c |   18 ------------------\n builtin.h           |    1 -\n 2 files changed, 0 insertions(+), 19 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex c2ec6ea..d288fde 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -45,24 +45,6 @@ static int is_from_line(const char *line, int len)\n \n static struct strbuf buf = STRBUF_INIT;\n \n-/* We cannot use fgets() because our lines can contain NULs */\n-int read_line_with_nul(char *buf, int size, FILE *in)\n-{\n-\tint len = 0, c;\n-\n-\tfor (;;) {\n-\t\tc = getc(in);\n-\t\tif (c == EOF)\n-\t\t\tbreak;\n-\t\tbuf[len++] = c;\n-\t\tif (c == '\\n' || len + 1 >= size)\n-\t\t\tbreak;\n-\t}\n-\tbuf[len] = '\\0';\n-\n-\treturn len;\n-}\n-\n /* Called with the first line (potentially partial)\n  * already in buf[] -- normally that should begin with\n  * the Unix \"From \" line.  Write it into the specified\ndiff --git a/builtin.h b/builtin.h\nindex 20427d2..9993dfc 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -13,7 +13,6 @@ extern const char git_more_info_string[];\n extern void list_common_cmds_help(void);\n extern const char *help_unknown_cmd(const char *cmd);\n extern void prune_packed_objects(int);\n-extern int read_line_with_nul(char *buf, int size, FILE *file);\n extern int fmt_merge_msg(int merge_summary, struct strbuf *in,\n \tstruct strbuf *out);\n extern int commit_tree(const char *msg, unsigned char *tree,\n-- \n1.6.4\n"},{"id":"119551","messageId":"r3l_p2g-BpVHWKE-kMWIRydBBfOVeq5L9RbpHa_iUxak9BjwONJobsuScOrzURDhi2RTa-0nDQY@cipher.nrlssc.navy.mil","threadId":"20361","inReplyTo":"r3l_p2g-BpVHWKE-kMWIR-hbNfTwGmUrXhM-lhoS9dZKPidslwcauSWyJW7X27JpF_HWaZz1tHI@cipher.nrlssc.navy.mil","subject":"[PATCH 4/4] Allow mailsplit (and hence git-am) to handle mails with CRLF line-endings","fromName":"Brandon Casey","fromEmail":"casey@nrlssc.navy.mil","sentAt":"2009-08-05T03:31:59Z","receivedAt":"2009-08-05T03:31:59Z","isPatch":true,"sender":{"key":"drafnel@gmail.com","avatar":"https://avatars.githubusercontent.com/u/921167?v=4"},"body":"From: Junio C Hamano <gitster@pobox.com>\n\nIt is not that uncommon to have mails with DOS line-ending, notably\nThunderbird and web mailers like Gmail (when saving what they call\n\"original\" message).  So modify mailsplit to convert CRLF line-endings to\njust LF.\n\nSince git-rebase is built on top of git-am, add an option to mailsplit to\nbe used by git-am when it is acting on behalf of git-rebase, to refrain\nfrom doing this conversion.\n\nAnd add a test to make sure that rebase still works.\n\nSigned-off-by: Brandon Casey <drafnel@gmail.com>\n---\n builtin-mailsplit.c |    6 ++++++\n git-am.sh           |    8 +++++++-\n t/t3400-rebase.sh   |   26 ++++++++++++++++++++++++--\n 3 files changed, 37 insertions(+), 3 deletions(-)\n\ndiff --git a/builtin-mailsplit.c b/builtin-mailsplit.c\nindex d288fde..c9c85c3 100644\n--- a/builtin-mailsplit.c\n+++ b/builtin-mailsplit.c\n@@ -44,6 +44,7 @@ static int is_from_line(const char *line, int len)\n }\n \n static struct strbuf buf = STRBUF_INIT;\n+static int keep_cr;\n \n /* Called with the first line (potentially partial)\n  * already in buf[] -- normally that should begin with\n@@ -69,6 +70,12 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n \t * \"From \" and having something that looks like a date format.\n \t */\n \tfor (;;) {\n+\t\tif (!keep_cr && buf.len > 1 && buf.buf[buf.len-1] == '\\n' &&\n+\t\t\tbuf.buf[buf.len-2] == '\\r') {\n+\t\t\tstrbuf_setlen(&buf, buf.len-2);\n+\t\t\tstrbuf_addch(&buf, '\\n');\n+\t\t}\n+\n \t\tif (fwrite(buf.buf, 1, buf.len, output) != buf.len)\n \t\t\tdie_errno(\"cannot write output\");\n \n@@ -226,6 +233,8 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)\n \t\t\tnr = strtol(arg+2, NULL, 10);\n \t\t} else if ( arg[1] == 'b' && !arg[2] ) {\n \t\t\tallow_bare = 1;\n+\t\t} else if (!strcmp(arg, \"--keep-cr\")) {\n+\t\t\tkeep_cr = 1;\n \t\t} else if ( arg[1] == 'o' && arg[2] ) {\n \t\t\tdir = arg+2;\n \t\t} else if ( arg[1] == '-' && !arg[2] ) {\ndiff --git a/git-am.sh b/git-am.sh\nindex d64d997..985226b 100755\n--- a/git-am.sh\n+++ b/git-am.sh\n@@ -197,7 +197,13 @@ check_patch_format () {\n split_patches () {\n \tcase \"$patch_format\" in\n \tmbox)\n-\t\tgit mailsplit -d\"$prec\" -o\"$dotest\" -b -- \"$@\" > \"$dotest/last\" ||\n+\t\tcase \"$rebasing\" in\n+\t\t'')\n+\t\t\tkeep_cr= ;;\n+\t\t?*)\n+\t\t\tkeep_cr=--keep-cr ;;\n+\t\tesac\n+\t\tgit mailsplit -d\"$prec\" -o\"$dotest\" -b $keep_cr -- \"$@\" > \"$dotest/last\" ||\n \t\tclean_abort\n \t\t;;\n \tstgit-series)\ndiff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh\nindex c5c29cc..4e6a44b 100755\n--- a/t/t3400-rebase.sh\n+++ b/t/t3400-rebase.sh\n@@ -3,9 +3,10 @@\n # Copyright (c) 2005 Amos Waterland\n #\n \n-test_description='git rebase should not destroy author information\n+test_description='git rebase assorted tests\n \n-This test runs git rebase and checks that the author information is not lost.\n+This test runs git rebase and checks that the author information is not lost\n+among other things.\n '\n . ./test-lib.sh\n \n@@ -133,4 +134,25 @@ test_expect_success 'rebase -q is quiet' '\n      test ! -s output.out\n '\n \n+q_to_cr () {\n+\ttr Q '\\015'\n+}\n+\n+test_expect_success 'Rebase a commit that sprinkles CRs in' '\n+\t(\n+\t\techo \"One\"\n+\t\techo \"TwoQ\"\n+\t\techo \"Three\"\n+\t\techo \"FQur\"\n+\t\techo \"Five\"\n+\t) | q_to_cr >CR &&\n+\tgit add CR &&\n+\ttest_tick &&\n+\tgit commit -a -m \"A file with a line with CR\" &&\n+\tgit tag file-with-cr &&\n+\tgit checkout HEAD^0 &&\n+\tgit rebase --onto HEAD^^ HEAD^ &&\n+\tgit diff --exit-code file-with-cr:CR HEAD:CR\n+'\n+\n test_done\n-- \n1.6.4\n"},{"id":"119578","messageId":"81b0412b0908042249g23f7487eo5bf1b9c9027024c@mail.gmail.com","threadId":"20361","inReplyTo":"r3l_p2g-BpVHWKE-kMWIRydBBfOVeq5L9RbpHa_iUxak9BjwONJobsuScOrzURDhi2RTa-0nDQY@cipher.nrlssc.navy.mil","subject":"Re: [PATCH 4/4] Allow mailsplit (and hence git-am) to handle mails with CRLF line-endings","fromName":"Alex Riesen","fromEmail":"raa.lkml@gmail.com","sentAt":"2009-08-05T05:49:10Z","receivedAt":"2009-08-05T05:49:10Z","isPatch":true,"sender":{"key":"raa.lkml@gmail.com","avatar":"https://avatars.githubusercontent.com/u/324101?v=4"},"body":"On Wed, Aug 5, 2009 at 05:31, Brandon Casey<casey@nrlssc.navy.mil> wrote:\n> @@ -69,6 +70,12 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)\n>         * \"From \" and having something that looks like a date format.\n>         */\n>        for (;;) {\n> +               if (!keep_cr && buf.len > 1 && buf.buf[buf.len-1] == '\\n' &&\n> +                       buf.buf[buf.len-2] == '\\r') {\n> +                       strbuf_setlen(&buf, buf.len-2);\n> +                       strbuf_addch(&buf, '\\n');\n> +               }\n> +\n\nThat's much better then any on-the-fly corrections :)\n"},{"id":"119722","messageId":"alpine.LSU.2.00.0908060009400.15736@hermes-2.csi.cam.ac.uk","threadId":"20361","inReplyTo":"40aa078e0908041359p79b111eo2f389dde25bb0876@mail.gmail.com","subject":"Re: Please make git-am handle \\r\\n-damaged patches","fromName":"Tony Finch","fromEmail":"dot@dotat.at","sentAt":"2009-08-05T23:10:30Z","receivedAt":"2009-08-05T23:10:30Z","isPatch":false,"sender":{"key":"dot@dotat.at","avatar":"https://avatars.githubusercontent.com/u/68429?v=4"},"body":"On Tue, 4 Aug 2009, Erik Faye-Lund wrote:\n>\n> RFC 2822, section 2.3 explicitly states that a CR should not occur\n> without a LF (and vice versa, but the e-mail client might convert CRLF\n> to LF when saving to file), so I think this should be safe.\n\nThe rare BINARYMIME extension relaxes this requirement.\n\nTony.\n-- \nf.anthony.n.finch  <dot@dotat.at>  http://dotat.at/\nGERMAN BIGHT HUMBER: SOUTHWEST 5 TO 7. MODERATE OR ROUGH. SQUALLY SHOWERS.\nMODERATE OR GOOD.\n"}]}