On Wed, Mar 9, 2016 at 1:49 AM, Junio C Hamano <gits...@pobox.com> wrote: > Karthik Nayak <karthik....@gmail.com> writes: > >> On Tue, Mar 8, 2016 at 7:26 AM, Jacob Keller <jacob.kel...@gmail.com> wrote: >>> On Mon, Mar 7, 2016 at 3:08 PM, Junio C Hamano <gits...@pobox.com> wrote: >>>> Karthik Nayak <karthik....@gmail.com> 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.
That's actually a very good point, will incorporate that, thanks! -- Regards, Karthik Nayak -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
