Stefan Beller <sbeller@xxxxxxxxxx> writes: > However if we add a value restriction here, we need to be as strict in the > .gitattributes parsing as well and put a warning there (similar to > invalid_attr_name_message) I would think. Remember, the attribute system is used for many purposes other than this new "further limit pathspec". While the (hopefully temporary) limitation hurts here, users will limit the attributes they use for ":(attr:VAR=VAL)" pathspec magic, and it is unlikely that either the VAR part or its VALue are something the core Git currently uses anyway. So I do not think it is necessary or even beneficial to add such a warning. > +static char *attr_value_unescape(const char *value) > +{ > + const char *src; > + char *dst, *ret; > + > + ret = xmallocz(strlen(value)); > + for (src = value, dst = ret; *src; src++, dst++) { > + if (*src == '\\') { > + if (!src[1]) > + die(_("Escape character '\\' not allowed as " > + "last character in attr value")); > + src++; > + } > + if (*src && invalid_value_char(*src)) > + die("cannot use '%c' for value matching", *src); > + *dst = *src; > + } > + *dst = '\0'; > + return ret; > +} Please sanity-check me. Just like I said to your original "I doubt *i could be NUL here", I now doubt *src could be NUL there where invalid_value_char() gets called. If *src could be NUL there, then *dst gets NUL once, and then after loop exits (presumably after incrementing dst), *dst gets another NUL, which was the terminating NUL condition being iffy I mentioned, but as you said, I do not think it would happen, so we can lose the "*src && " before invalid_value_char() is called. 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