Eric Blake <ebl...@redhat.com> writes:
> On 9/10/19 1:37 AM, Markus Armbruster wrote:
>> Since the previous commit restricted strings to printable ASCII,
>> \uXXXX's only use is obfuscation. Drop it.
>>
>> This leaves \\, \/, \', and \". Since QAPI schema strings are all
>> names, and names are restricted to ASCII letters, digits, hyphen, and
>> underscore, none of them is useful.
>>
>> The latter three have no test coverage. Drop them.
>>
>> Keep \\ to avoid (more) gratuitous incompatibility with JSON.
>
> We have to parse it (to get a sane error message for someone writing
> "a\b" and thinking they were getting a backspace), but we can still
> reject "a\\b" as being a non-QAPI-name. An alternative might be to
> reject QAPI strings the moment \ is encountered (as the only possible
> use is to introduce a character that is not valid as a name), to the
> point that we could check for name validity at the time we parse strings
> rather than later on. Up to you.
That works, too. I chose to parse \\ to keep the language a
sub-language of JSON's (and Python's) at the lexical level: we reject
some valid JSON (and Python) such as 'a\b'. Not treating \ special at
all would accept it, but with a different meaning. Will be rejected at
a higher level, so it doesn't really matter. But the reasoning involves
more than just the lexical level then.
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>
>
>> --- a/tests/qapi-schema/escape-outside-string.err
>> +++ /dev/null
>> @@ -1 +0,0 @@
>> -tests/qapi-schema/escape-outside-string.json:3:27: Stray "\"
>
> Do we still want to test that \ is an invalid character outside of
> strings (whether or not \ is also made invalid inside strings)?
funny-char.json checks ';'. '\' is no different.
>> +++ b/tests/qapi-schema/unknown-escape.json
>> @@ -1,3 +1,3 @@
>> -# we only recognize JSON escape sequences, plus our \' extension (no \x)
>> +# we only recognize \\
>> # { 'command': 'foo', 'data': {} }
>> { 'command': 'foo', 'dat\x61':{} }
>>
>
> At any rate, this change seems reasonable.
>
> Reviewed-by: Eric Blake <ebl...@redhat.com>
Thanks!
