Re: [PATCH 08/15] ref-filter: make "%(symref)" atom work with the ':short' modifier

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

 



Karthik Nayak <karthik.188@xxxxxxxxx> writes:

> On Tue, Mar 8, 2016 at 7:26 AM, Jacob Keller <jacob.keller@xxxxxxxxx> wrote:
>> On Mon, Mar 7, 2016 at 3:08 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
>>> Karthik Nayak <karthik.188@xxxxxxxxx> writes:
>>>
>>>> The "%(symref)" atom doesn't work when used with the ':short' modifier
>>>> because we strictly match only 'symref' for setting the 'need_symref'
>>>> indicator. Fix this by using 'starts_with()' rather than 'strcmp()'.
>>>
>>> Does that mean you also accept %(symrefgarbage) without complaining?
>>>
>>>
>>
>> Looks like patch 9 fixes this by introducing symref_atom_parser.
>>
>
> There are two ways this kinda errors can occur:
> 1. %(symrefgarbage) : This is handled by parse_ref_filter_atom() which would
> print a "fatal: unknown field name: symrefgarbage".
> 2. %(symref:garbage): This is handled by populate_value() which would print
> a "fatal: unknown symref: format garbage".
>
> Either ways we do not need to worry about this as existing code would handle
> it. Also like Jacob mentioned Patch 09 would ensure this error checking would
> happen within symref_atom_parser().

You forgot to mention that there is 3., which is that you just
closed the door for a new valid_atom[] that begins with substring
"symref" which does not need to flip need_symref on, I think.

You can check valid_atom[i].name with strcmp() to achieve what you
are trying to do here, instead of checking used_atom[at].name, and I
think that would be a cleaner way to avoid all three problems.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



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