| Message ID | 1434120674-8122-9-git-send-email-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/12/2015 08:51 AM, Markus Armbruster wrote: > Screwed up in commit e53188a. > And partly my fault for taking a patch written in python by someone else, without being a python guru myself. :) > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > scripts/qapi.py | 26 ++++++++++++-------------- > 1 file changed, 12 insertions(+), 14 deletions(-) > Is it worth a testsuite enhancement to expose this? > + elif self.src.startswith("true", self.pos): > + self.val = True > + self.cursor += 3 > + return > + elif self.src.startswith("false", self.pos): We still parse things like bare 'truest' as the token 'true' concatenated with the nonsense 'st', which is probably not the nicest of error messages, but the chances of someone mistyping bare words is not worth making it more robust. Reviewed-by: Eric Blake <eblake@redhat.com>
Eric Blake <eblake@redhat.com> writes: > On 06/12/2015 08:51 AM, Markus Armbruster wrote: >> Screwed up in commit e53188a. >> > > And partly my fault for taking a patch written in python by someone > else, without being a python guru myself. :) > >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> scripts/qapi.py | 26 ++++++++++++-------------- >> 1 file changed, 12 insertions(+), 14 deletions(-) >> > > Is it worth a testsuite enhancement to expose this? Four existing tests trigger the same error message: * bad-type-int.err enum-int-member.err Only because we neglect to parse numbers. * escape-outside-string.err funny-char.err These test stray '\' and ';'. I guess a separate test covering bad literal names would be useful if we actually do something different there. >> + elif self.src.startswith("true", self.pos): >> + self.val = True >> + self.cursor += 3 >> + return >> + elif self.src.startswith("false", self.pos): > > We still parse things like bare 'truest' as the token 'true' > concatenated with the nonsense 'st', which is probably not the nicest of > error messages, but the chances of someone mistyping bare words is not > worth making it more robust. > > Reviewed-by: Eric Blake <eblake@redhat.com> What regular expression should the lexer consume for a literal name? My patch consumes (true|false|null). For what it's worth, qobject/json-{lexer,parser}.c consume [a-z]+, then reject anything but true, false, null. Lexes false0 as two tokens JSON_KEYWORD false and JSON_INTEGER 0. Can't find a context where the parser accepts that pair. It rejects it with its usual garbage error message "Invalid JSON syntax".
diff --git a/scripts/qapi.py b/scripts/qapi.py index a24a7e2..6faa897 100644 --- a/scripts/qapi.py +++ b/scripts/qapi.py @@ -217,20 +217,18 @@ class QAPISchema: return else: string += ch - elif self.tok in "tfn": - val = self.src[self.cursor - 1:] - if val.startswith("true"): - self.val = True - self.cursor += 3 - return - elif val.startswith("false"): - self.val = False - self.cursor += 4 - return - elif val.startswith("null"): - self.val = None - self.cursor += 3 - return + elif self.src.startswith("true", self.pos): + self.val = True + self.cursor += 3 + return + elif self.src.startswith("false", self.pos): + self.val = False + self.cursor += 4 + return + elif self.src.startswith("null", self.pos): + self.val = None + self.cursor += 3 + return elif self.tok == '\n': if self.cursor == len(self.src): self.tok = None
Screwed up in commit e53188a. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- scripts/qapi.py | 26 ++++++++++++-------------- 1 file changed, 12 insertions(+), 14 deletions(-)