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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jun 3, 2016 at 8:02 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Fri, Jun 3, 2016 at 7:42 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>> Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
>>
>>>>         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.
>>
>> Hmph, none of ">", "F", "r", "o", "m", or " " is "\n"; would eol
>> still be an issue?
>
> Ah right. Sorry for being slow.
>
> Your concise version of is_mboxrd_from() is a nice alternative, as well.

My only minor reservation is that it your concise version is still
subtle with regard to not taking 'linelen' into account. At first
glance, it looks like a bug that it doesn't consider the logical
end-of-line, and someone reading the code has to put in extra effort
to convince himself that the code works as intended.

For that reason, I have a bit of a preference for a version of
is_mboxrd_from() which does take 'linelen' into account explicitly.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]