Re: Segfault in the attr stack
On Wed, Jun 1, 2016 at 3:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 13 quoted lines
>
> By the way, I just noticed that the <specification> part of the
> ':(attr:<specification>)' syntax would need to be rethought. In the
> .gitattributes file everybody has, we see these lines:
>
> *.[ch] whitespace=indent,trail,space
> *.sh whitespace=indent,trail,space
>
> but because comma is a special separator in the pathspec magic
> system, we cannot do
>
> $ git status ':(attr:whitespace=indent,trail,space)'
>
Show 17 quoted lines
>>
>>> + if (!item->attr_check)
>>> + item->attr_check = git_attr_check_alloc();
>>
>> Given more than one "attr" magic, e.g. ":(attr:A=a,attr:B)/path",
>> the check may not be empty when we process the second one; we just
>> extend it without losing the existing contents.
>
> That is why I am not super happy with it though.
>
> ":(attr:A=a,attr:B)/path",
> ":(attr:A=a B)/path",
>
> are the same for the user as well as in the internal data structures.
> This "wastes" the white space as a possible convenient separator
> character, e.g. for multiple values. On the other hand it will be easier
> to type, specially for many attrs (read submodule groups).
So at that time I thought I had communicated the issue enough and we'd be fine ignoring it for now. I propose to not escape commas, but use white spaces instead, i.e.
git status ':(attr:whitespace=indent trail space,attr:label=with
more values)' ':attr(attr:foo:bar)'
would match
* all files that have the whitespace AND the label setting (matching
exactly the values)
* OR foo=bar attribute
This syntax would require to repeat ",attr:" for multiple ANDed attributes, but would save us from escaped commas, which may be a pain both in parsing as well as doing the input on a shell?
[1] http://thread.gmane.org/gmane.comp.version-control.git/294989/focus=295016
Show 21 quoted lines
> I think we should introduce a quoting mechanism to hide these commas
> from the pathspec magic splitter, e.g.
>
> where attr_value_unquote() would copy string while unquoting some
> special characters (i.e. at least ' ' and ',' because they are used
> as syntactic elements in the higher level; there might be others).
>
> diff --git a/pathspec.c b/pathspec.c
> index 0a02255..fb22f28 100644
> --- a/pathspec.c
> +++ b/pathspec.c
> @@ -132,7 +132,7 @@ static void parse_pathspec_attr_match(struct pathspec_item *item, const char *va
> am->match_mode = MATCH_SET;
> else {
> am->match_mode = MATCH_VALUE;
> - am->value = xstrdup(&attr[attr_len + 1]);
> + am->value = attr_value_unquote(&attr[attr_len + 1]);
> if (strchr(am->value, '\\'))
> die(_("attr spec values must not contain backslashes"));
> }
>If we go by whitespaces, we can implement attr_value_unquote as a `tr " " ","` conceptually, which seems easy.
Thanks, Stefan
Stefan Beller
