Re: [PATCH v4 5/8] trailer: clarify failure modes in parse_trailer

From: Jonathan Tan <jonathantanmy@google.com>
Date: Oct 20, 2016, 22:49 UTC
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:

Show 16 quoted lines

> 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.