Junio C Hamano <gitster@xxxxxxxxx> writes:
> "Hariom Verma via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:
>
>> From: Hariom Verma <hariom18599@xxxxxxxxx>
>>
>> The 'contents' atom does not show any error if used with 'trailers'
>> atom and semicolon is missing before trailers arguments.
>>
>> e.g %(contents:trailersonly) works, while it shouldn't.
>>
>> It is definitely not an expected behavior.
>>
>> Let's fix this bug.
>>
>> Mentored-by: Christian Couder <chriscool@xxxxxxxxxxxxx>
>> Mentored-by: Heba Waly <heba.waly@xxxxxxxxx>
>> Signed-off-by: Hariom Verma <hariom18599@xxxxxxxxx>
>> ---
>
> Nice spotting. 7a5edbdb (ref-filter.c: parse trailers arguments
> with %(contents) atom, 2017-10-01) talks about being deliberate
> about the case where skip_prefix(":") does not find a colon after
> the "trailers" token, but from the message it is clear that it
> expected that the case happens only when "trailers" is at the end of
> the string.
>
> The new helper that is overly verbose and may be overkill.
>
> Shouldn't this be clear enough, equivalent and sufficient?
>
> else if (skip_prefix(arg, "trailers", &arg) &&
> (!*arg || *arg == ':'))) {
> if (trailers_atom_parser(...);
Ah, no, even with "*arg++ == ':'. This moves arg past "trailers" if
given "trailersandsomegarbage" and the next one in "else if" cascade
would look at "andsomegarbage"---which is not what we want.
>> +static int check_format_field(const char *arg, const char *field, const char **option)
>> +{
>> + const char *opt;
>> + if (skip_prefix(arg, field, &opt)) {
>> + if (*opt == '\0') {
>> + *option = NULL;
>> + return 1;
>> + }
>> + else if (*opt == ':') {
>> + *option = ++opt;
>> + return 1;
>> + }
>> + }
>> + return 0;
>> +}
And the helper does not have such a breakage. It looks good.
Thanks.
