Re: [PATCH v2 01/10] sequencer.c: remove broken support for rfc2822 continuation in footer

From: Brandon Casey <drafnel@gmail.com>
Date: Jan 22, 2013, 11:08 UTC
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:

Show 16 quoted lines

> 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