Re: [PATCHv3] pathspec: allow escaped query values

> Stefan Beller <sbeller@google.com> 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".

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