From: Brandon Casey <drafnel@gmail.com>
Date: Tue, 22 Jan 2013 11:08:40 GMT
Subject: Re: [PATCH v2 01/10] sequencer.c: remove broken support for rfc2822 continuation in footer
Message-ID: <CA+sFfMeB_PEgoykPj-RX6zZ-hOD1106aO_KwkWF7BBiJvG-1AA@mail.gmail.com>
In-Reply-To: <20130122094916.GB8908@elie.Belkin>

On Tue, Jan 22, 2013 at 1:49 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Jonathan Nieder wrote:
>
>> Here is the loop in master:
>>
>>       int hit = 0;
>> [...]
>>
>>       for (i = len - 1; i > 0; i--) {
>>               if (hit && buf[i] == '\n')
>>                       break;
>>               hit = (buf[i] == '\n');
>>       }
>>
>> I don't see any adjacency check.
>
> Of course that's because I can't read. :)  Checking again.

Blame the code, not your eyes.  The use of the term 'hit' is what
makes this loop confusing.  It gives the impression that 'hit' gets
set once, after the first newline is /hit/.

It would be much easier to read if it was written like this:

   int last_char_was_nl = 0;
   for (i = len - 1; i > 0; i--) {
           this_char_is_nl = (buf[i] == '\n');
           if (last_char_was_nl && this_char_is_nl)
                   break;
           last_char_was_nl = this_char_is_nl;
   }

I'll slide this in when I resubmit this series with your suggestions.

-Brandon