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

2016-10-22 Thread Junio C Hamano
Christian Couder writes: > On Fri, Oct 21, 2016 at 2:18 AM, Junio C Hamano wrote: >> >> If I were guiding a topic that introduce this feature from scratch >> today, I would probably suggest a pattern based approach, e.g. a >> built-in

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

2016-10-22 Thread Christian Couder
On Fri, Oct 21, 2016 at 2:18 AM, Junio C Hamano wrote: > Jonathan Tan writes: > >> 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),

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

2016-10-22 Thread Christian Couder
On Fri, Oct 21, 2016 at 12:45 AM, Junio C Hamano wrote: > Jonathan Tan 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

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

2016-10-20 Thread Junio C Hamano
Jonathan Tan writes: > 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

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

2016-10-20 Thread Jonathan Tan
On 10/20/2016 03:45 PM, Junio C Hamano wrote: Jonathan Tan 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.

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

2016-10-20 Thread Junio C Hamano
Jonathan Tan 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

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

2016-10-20 Thread Jonathan Tan
On 10/20/2016 03:40 PM, Jonathan Tan wrote: On 10/20/2016 03:14 PM, Junio C Hamano wrote: Stefan Beller writes: +static int find_separator(const char *line) +{ + const char *c; + for (c = line; ; c++) { + if (!*c || *c == '\n') +

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

2016-10-20 Thread Jonathan Tan
On 10/20/2016 03:14 PM, Junio C Hamano wrote: Stefan Beller writes: +static int find_separator(const char *line) +{ + const char *c; + for (c = line; ; c++) { + if (!*c || *c == '\n') + return -1; + if (*c ==

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

2016-10-20 Thread Junio C Hamano
Stefan Beller 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))

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

2016-10-20 Thread Stefan Beller
On Thu, Oct 20, 2016 at 2:39 PM, Jonathan Tan wrote: > The parse_trailer function has a few modes of operation, all depending > on whether the separator is present in its input, and if yes, the > separator's position. Some of these modes are failure modes, and these >

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

2016-10-20 Thread Jonathan Tan
The parse_trailer function has a few modes of operation, all depending on whether the separator is present in its input, and if yes, the separator's position. Some of these modes are failure modes, and these failure modes are handled differently depending on whether the trailer line was sourced