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

Matthieu Moy [off-list ref] writes:

Junio C Hamano [off-list ref] writes:

quoted

Karthik Nayak [off-list ref] writes:

quoted

-		} 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.

Yeah, it can be a separate topic.  I am neutral (i.e. I certainly
would not insist that the existing one should be fixed with the
helper in the series, but I cannot quite say that I prefer the fix
to be made outside this topic, either).

Thanks.