On Thu, Jun 2, 2016 at 4:23 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> 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".
Right, and in the past we followed a rigid pattern of non-crazy values.
The crazy values will show up once users realize they can
put anything they want into the .gitattributes to query for files.
Before this labeling change, there was no need to put anything
other the officially documented values into the attribute files.
> So I do not think it is necessary or even beneficial to add such a
> warning.
Ok.
>
>> +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.
Right, we can lose the *src check before invalid_value_char
as that ought to be caught in if (!src[1]) die(...) before.
Thanks,
Stefan
>
> 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