Re: [PATCHv7 5/5] pathspec: allow querying for attributes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Stefan Beller <sbeller@xxxxxxxxxx> writes:

> and those are not yet codified, but for discussion then:
>
>  - "`?`" the attribute must not be unspecified, i.e. set, unset or has any value
>  - "`+`" the attribute must be set or has any value

I'd suggest not to support these until we see any concrete and
convincing example use case to tell us why this is useful.

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

You can interpret the former as ORed :(attr:A=a) and :(attr:B) if
you really wanted to.  You left the door open for such a future
extension by explicitly rejecting multiple attributes added to a
single pathspec element in earlier round, which was good.  You could
do the same here if you do not want to code that ORed ANDs, so that
it (or some other semantics) can be introduced later without breaking
the mental model the users would form with the initial implementation.

>>> +             val_len = strcspn(val, "=,)");
>>
>> I understand "=", but can "," and ")" appear here?
>
> This was overly caution from some intermediate state, where the caller
> handed in more than required.

Is that being overly cautious?

It looks to me more like being sloppy and sweeping bugs in callers
that you could have diagnosed here.

If you didn't have ",)" in the cspn set above, at least you would
see the effect caused by a broken caller.  E.g. ":(attr:FOO,icase)"
would be given to you as "FOO,icase)" by a broken caller, you would
split "FOO,icase" as one of the SP separated items, and try to parse
it as an attribute name.  You can notice the breakage of the caller
at that time.  With ",)" in cspn set, you silently pass the parameter
given by a broken caller.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]