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

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

 



On 10/20/2016 03:40 PM, Jonathan Tan wrote:
On 10/20/2016 03:14 PM, Junio C Hamano wrote:
Stefan Beller <sbeller@xxxxxxxxxx> 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.)

Actually I guess we could generate the separators_and_equal string whenever we obtain new separators from the config. I'll do this in the next reroll.



[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]