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

> On 10/20/2016 03:14 PM, Junio C Hamano wrote:
>> Stefan Beller <sbeller@google.com> writes:
>>
>>>> +static int find_separator(const char *line)
>>>> +{
>>>> +       const char *c;
>>>> +       for (c = line; ; c++) {
>>>> +               if (!*c || *c == '\n')
>>>> +                       return -1;
>>>> +               if (*c == '=' || strchr(separators, *c))
>>>> +                       return c - line;
>>>> +       }
>>>
>>> I was about to suggest this function can be simplified and maybe
>>> even inlined by the use of strspn or strcspn, but I think manual
>>> processing of the string is fine, too, as it would not really be
>>> shorter.
>>
>> Hmm, I fear that iterating over a line one-byte-at-a-time and
>> running strchr(separators, *c) on it for each byte has a performance
>> implication over running a single call to strcspn(line, separators).
>
> 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.)