Re: [PATCH v17 05/14] ref-filter: introduce match_atom_name()

From: Matthieu Moy <matthieu.moy@grenoble-inp.fr>
Date: Sep 10, 2015, 16:59 UTC
Message-ID: <vpqio7inrc5.fsf@scolette.imag.fr>
In-Reply-To: <xmqq37ym5i2u.fsf@gitster.mtv.corp.google.com>

Junio C Hamano <gitster@pobox.com> writes:

Show 17 quoted lines

> Karthik Nayak <karthik.188@gmail.com> writes:
>
>> -		} else if (starts_with(name, "color:")) {
>> +		} else if (match_atom_name(name, "color", &valp)) {
>
> Why use the helper only for this one?  Aren't existing calls to
> starts_with() in the same if/else if/... cascade all potential bugs
> that the new helper function is meant to help fixing?  For example,
> the very fist one in the cascade:
>
> 	if (starts_with(name, "refname"))
>         	refname = ref->refname;
>
> is correct *ONLY* when name is "refname" or "refname:" followed by
> something, and it should skip "refnamex" when such a new atom is
> added to valid_atom[] list, i.e. a bug waiting to happen.  I think
> the new helper is designed to prevent such a bug from happening.

I fully agree, but I also think that this should be a separate topic.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/