Matthieu Moy <Matthieu.Moy@xxxxxxxxxxxxxxx> writes:
> Junio C Hamano <gitster@xxxxxxxxx> writes:
>
>> Karthik Nayak <karthik.188@xxxxxxxxx> 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.
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.
--
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