Re: [PATCH 1/3] pretty: support "mboxrd" output format

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jun 3, 2016, 22:59 UTC
Message-ID: <CAPig+cQXA3mgYC3rCTx2h93rB6djdKvYJ+oCXUP24-xk2PqW=Q@mail.gmail.com>
In-Reply-To: <xmqqlh2loq6p.fsf@gitster.mtv.corp.google.com>

On Fri, Jun 3, 2016 at 6:36 PM, Junio C Hamano <gitster@pobox.com> wrote:

Show 32 quoted lines

> Eric Sunshine <sunshine@sunshineco.com> writes:
>> On Thu, Jun 2, 2016 at 3:51 AM, Eric Wong <e@80x24.org> wrote:
>>> Eric Wong <e@80x24.org> wrote:
>>>> Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> > On Tue, May 31, 2016 at 3:45 AM, Eric Wong <e@80x24.org> wrote:
>>>> > > Eric Sunshine <sunshine@sunshineco.com> wrote:
>>>> > >> I wonder if hand-coding, rather than using a regex, could be an improvement:
>>>> > >>
>>>> > >>     static int is_mboxrd_from(const char *s, size_t n)
>>>> > >>     {
>>>> > >>         size_t f = strlen("From ");
>>>> > >>         const char *t = s + n;
>>>> > >>
>>>> > >>         while (s < t && *s == '>')
>>>> > >>             s++;
>>>> > >>         return t - s >= f && !memcmp(s, "From ", f);
>>>> > >>     }
>>>> > >>
>>>> > >> or something.
>>>> > >
>>>> > > Yikes.  I mostly work in high-level languages and do my best to
>>>> > > avoid string parsing in C; so that scares me.  A lot.
>>
>> As mentioned above, it's all subjective and, of course, I have a bias
>> toward the example I provided, but don't otherwise feel strongly about
>> it. I do, however, like the idea of using a simple hand-coded matching
>> function over the regex (but no so much that I would complain about
>> it). Use whatever you and Junio feel is appropriate.
>
> This is meant to be a replacement for the original that uses
> regexec(), which in turn means the string we are checking is
> guaranteed to be NUL terminated, right?

Yes, this is meant as a replacement for the regexec(), but no, there is no guarantee that the "line" is NUL-terminated. In this case, the 'line' in pretty.c:pp_remainder() might end at a NUL or it might just end at the next newline in the larger buffer.

The bit about sending the "line" to regexec() without taking its logical end-of-line into account was what caught my attention in the first place.

Show 5 quoted lines

>         static int is_mboxrd_from(const char *line) {
>                 return starts_with(line + strspn(line, ">"), "From ");
>         }
>
> is sufficiently high-level that no longer is scary, hopefully?

That's nice and concise but unfortunately not useful for this case where we must respect the logical end-of-line (within the larger buffer), represented by line+linelen.