From: Jonathan Tan <jonathantanmy@google.com>
Date: Thu, 20 Oct 2016 22:49:33 GMT
Subject: Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer
Message-ID: <5656444f-14ba-b58c-4ad6-e72cc15a13cb@google.com>
In-Reply-To: <xmqqr37aej26.fsf@gitster.mtv.corp.google.com>

On 10/20/2016 03:45 PM, Junio C Hamano wrote:
> Jonathan Tan <jonathantanmy@google.com> writes:
>
>> If we do that, there is also the necessity of creating a string that
>> combines the separators and '=' (I guess '\n' is not necessary now,
>> since all the lines are null terminated). I'm OK either way.
>>
>> (We could cache that string, although I would think that if we did
>> that, we might as well write the loop manually, like in this patch.)
>
> I wonder if there is a legit reason to look for '=' in the first
> place.  "Signed-off-by= Jonathan Tan <jt@my.home>" does not look
> like a valid trailer line to me.
>
> Isn't that a remnant of lazy coding in the original that tried to
> share a single parser for contents and command line options or
> something?

That is true - I think we can take the allowed separators as an argument 
(meaning that we can have different behavior for file parsing and 
command line parsing), and since we already have that string, we can use 
strcspn. I'll try this out in the next reroll.