Re: [Qemu-devel] [PATCH v2 1/2] qapi: Allow decimal values

[Markus Armbruster <armbru@redhat.com>]

Eric Blake <eblake@redhat.com> writes:

> On 04/29/2014 03:44 AM, Fam Zheng wrote:
>> This allows giving decimal constants in the schema as expr.
>> 
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>>  scripts/qapi.py | 15 +++++++++++++--
>>  1 file changed, 13 insertions(+), 2 deletions(-)
>> 
>> diff --git a/scripts/qapi.py b/scripts/qapi.py
>> index b474c39..6022de5 100644
>> --- a/scripts/qapi.py
>> +++ b/scripts/qapi.py
>> @@ -108,6 +108,14 @@ class QAPISchema:
>>                          return
>>                      else:
>>                          string += ch
>> +            elif self.tok in "123456789":
>> +                self.val = int(self.tok)
>> +                while True:
>> +                    ch = self.src[self.cursor]
>> +                    if ch not in "1234567890":
>> +                        return
>> +                    self.val = self.val * 10 + int(ch)
>> +                    self.cursor += 1
>
> What does this do on integer overflow?  Should you validate that the
> result fits in [u]int64_t?  Should you allow for a leading '-' sign for
> a default of a negative number?  You have forbidden '0' as a valid
> number (although you correctly forbid '00' and any non-zero number with
> a leading 0, which matches JSON restrictions).

If every valid JSON number is also a valid Python number, then you can
accumulate the token, then convert it with built-in function int().
Just like our C JSON lexer uses strtoll(), in parse_literal().

>> -        if not self.tok in [ '{', '[', "'" ]:
>> -            raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
>> +        if not self.tok in "{['123456789":
>> + raise QAPISchemaError(self, 'Expected "{", "[", "]", string or
>> number')
>
> Again, this forbids the use of '0' as a default.

The spec for our lexical analysis comes from RFC 4627.  We take some
liberties with it, but we should do so only deliberately, and with a
sensible reason.  We should also keep the Python version (qapi.py)
consistent with the C version (qobject/json*.[ch]).

In particular see how parse_literal() deals with integer overflow.

Grammar for numbers, straight from RFC 4627:

     number = [ minus ] int [ frac ] [ exp ]

         decimal-point = %x2E       ; .

         digit1-9 = %x31-39         ; 1-9

         e = %x65 / %x45            ; e E

         exp = e [ minus / plus ] 1*DIGIT

         frac = decimal-point 1*DIGIT

         int = zero / ( digit1-9 *DIGIT )

         minus = %x2D               ; -

         plus = %x2B                ; +

         zero = %x30                ; 0

Since we distinguish between integral and fractional numbers everywhere
in our usage of JSON, it makes sense to do so here as well.

This means we want to accept this sub-grammar:

    inum = [ minus ] int

         minus = %x2D               ; -

         digit1-9 = %x31-39         ; 1-9

         zero = %x30                ; 0

         int = zero / ( digit1-9 *DIGIT )

json-lexer.c's state machine implements this faithfully (as far as I can
tell).

Fractional numbers could be left for later here, since your PATCH 2/2,
the first and so far only user of numbers, arbitrarily restricts them to
integer.  Just implementing them might be easier than documenting the
restriction, though...