* [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 12:48 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test Markus Armbruster
` (8 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
The parser handles erroneous input badly. To be improved shortly.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
configure | 2 +-
tests/Makefile | 22 ++++++++++++++++++++--
tests/qapi-schema/empty.exit | 1 +
tests/qapi-schema/empty.out | 3 +++
tests/qapi-schema/funny-char.exit | 1 +
tests/qapi-schema/funny-char.json | 2 ++
tests/qapi-schema/funny-char.out | 3 +++
tests/qapi-schema/indented-expr.exit | 1 +
tests/qapi-schema/indented-expr.json | 2 ++
tests/qapi-schema/indented-expr.out | 3 +++
tests/qapi-schema/missing-colon.exit | 1 +
tests/qapi-schema/missing-colon.json | 2 ++
tests/qapi-schema/missing-colon.out | 3 +++
tests/qapi-schema/missing-comma.exit | 1 +
tests/qapi-schema/missing-comma.json | 2 ++
tests/qapi-schema/missing-comma.out | 3 +++
tests/qapi-schema/non-objects.err | 1 +
tests/qapi-schema/non-objects.exit | 1 +
tests/qapi-schema/non-objects.json | 2 ++
tests/qapi-schema/quoted-structural-chars.exit | 1 +
tests/qapi-schema/quoted-structural-chars.json | 1 +
tests/qapi-schema/quoted-structural-chars.out | 3 +++
tests/qapi-schema/test-qapi.py | 25 +++++++++++++++++++++++++
tests/qapi-schema/unclosed-object.err | 1 +
tests/qapi-schema/unclosed-object.exit | 1 +
tests/qapi-schema/unclosed-object.json | 1 +
tests/qapi-schema/unclosed-string.err | 1 +
tests/qapi-schema/unclosed-string.exit | 1 +
tests/qapi-schema/unclosed-string.json | 2 ++
29 files changed, 90 insertions(+), 3 deletions(-)
create mode 100644 tests/qapi-schema/empty.err
create mode 100644 tests/qapi-schema/empty.exit
create mode 100644 tests/qapi-schema/empty.json
create mode 100644 tests/qapi-schema/empty.out
create mode 100644 tests/qapi-schema/funny-char.err
create mode 100644 tests/qapi-schema/funny-char.exit
create mode 100644 tests/qapi-schema/funny-char.json
create mode 100644 tests/qapi-schema/funny-char.out
create mode 100644 tests/qapi-schema/indented-expr.err
create mode 100644 tests/qapi-schema/indented-expr.exit
create mode 100644 tests/qapi-schema/indented-expr.json
create mode 100644 tests/qapi-schema/indented-expr.out
create mode 100644 tests/qapi-schema/missing-colon.err
create mode 100644 tests/qapi-schema/missing-colon.exit
create mode 100644 tests/qapi-schema/missing-colon.json
create mode 100644 tests/qapi-schema/missing-colon.out
create mode 100644 tests/qapi-schema/missing-comma.err
create mode 100644 tests/qapi-schema/missing-comma.exit
create mode 100644 tests/qapi-schema/missing-comma.json
create mode 100644 tests/qapi-schema/missing-comma.out
create mode 100644 tests/qapi-schema/non-objects.err
create mode 100644 tests/qapi-schema/non-objects.exit
create mode 100644 tests/qapi-schema/non-objects.json
create mode 100644 tests/qapi-schema/non-objects.out
create mode 100644 tests/qapi-schema/quoted-structural-chars.err
create mode 100644 tests/qapi-schema/quoted-structural-chars.exit
create mode 100644 tests/qapi-schema/quoted-structural-chars.json
create mode 100644 tests/qapi-schema/quoted-structural-chars.out
create mode 100644 tests/qapi-schema/test-qapi.py
create mode 100644 tests/qapi-schema/unclosed-object.err
create mode 100644 tests/qapi-schema/unclosed-object.exit
create mode 100644 tests/qapi-schema/unclosed-object.json
create mode 100644 tests/qapi-schema/unclosed-object.out
create mode 100644 tests/qapi-schema/unclosed-string.err
create mode 100644 tests/qapi-schema/unclosed-string.exit
create mode 100644 tests/qapi-schema/unclosed-string.json
create mode 100644 tests/qapi-schema/unclosed-string.out
diff --git a/configure b/configure
index 3d83e16..0a91434 100755
--- a/configure
+++ b/configure
@@ -4502,7 +4502,7 @@ if [ "$dtc_internal" = "yes" ]; then
fi
# build tree in object directory in case the source is not in the current directory
-DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos"
+DIRS="tests tests/tcg tests/tcg/cris tests/tcg/lm32 tests/libqos tests/qapi-schema"
DIRS="$DIRS pc-bios/optionrom pc-bios/spapr-rtas pc-bios/s390-ccw"
DIRS="$DIRS roms/seabios roms/vgabios"
DIRS="$DIRS qapi-generated"
diff --git a/tests/Makefile b/tests/Makefile
index cdbb79e..89a467b 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -83,6 +83,12 @@ gcov-files-arm-y += hw/tmp105.c
check-qtest-ppc-y += tests/boot-order-test$(EXESUF)
check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
+check-qapi-schema-y := $(addprefix tests/qapi-schema/, empty.json \
+ funny-char.json indented-expr.json missing-colon.json \
+ missing-comma.json non-objects.json \
+ quoted-structural-chars.json unclosed-object.json \
+ unclosed-string.json)
+
GENERATED_HEADERS += tests/test-qapi-types.h tests/test-qapi-visit.h tests/test-qmp-commands.h
test-obj-y = tests/check-qint.o tests/check-qstring.o tests/check-qdict.o \
@@ -171,6 +177,7 @@ check-help:
@echo " make check-qtest-TARGET Run qtest tests for given target"
@echo " make check-qtest Run qtest tests"
@echo " make check-unit Run qobject tests"
+ @echo " make check-qapi-schema Run QAPI schema tests"
@echo " make check-block Run block tests"
@echo " make check-report.html Generates an HTML test report"
@echo
@@ -233,13 +240,24 @@ check-report.html: check-report.xml
check-tests/qemu-iotests-quick.sh: tests/qemu-iotests-quick.sh qemu-img$(EXESUF) qemu-io$(EXESUF)
$<
+.PHONY: check-tests/test-qapi.py
+check-tests/test-qapi.py: tests/test-qapi.py
+
+.PHONY: $(patsubst %, check-%, $(check-qapi-schema-y))
+$(patsubst %, check-%, $(check-qapi-schema-y)): check-%.json: $(SRC_PATH)/%.json
+ $(call quiet-command, PYTHONPATH=$(SRC_PATH)/scripts $(PYTHON) $(SRC_PATH)/tests/qapi-schema/test-qapi.py <$^ >$*.out 2>$*.err; echo $$? >$*.exit, " TEST $*.out")
+ @diff -q $(SRC_PATH)/$*.out $*.out
+ @diff -q $(SRC_PATH)/$*.err $*.err
+ @diff -q $(SRC_PATH)/$*.exit $*.exit
+
# Consolidated targets
-.PHONY: check-qtest check-unit check
+.PHONY: check-qapi-schema check-qtest check-unit check
+check-qapi-schema: $(patsubst %,check-%, $(check-qapi-schema-y))
check-qtest: $(patsubst %,check-qtest-%, $(QTEST_TARGETS))
check-unit: $(patsubst %,check-%, $(check-unit-y))
check-block: $(patsubst %,check-%, $(check-block-y))
-check: check-unit check-qtest
+check: check-qapi-schema check-unit check-qtest
-include $(wildcard tests/*.d)
-include $(wildcard tests/libqos/*.d)
diff --git a/tests/qapi-schema/empty.err b/tests/qapi-schema/empty.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/empty.exit b/tests/qapi-schema/empty.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/empty.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/empty.json b/tests/qapi-schema/empty.json
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/empty.out b/tests/qapi-schema/empty.out
new file mode 100644
index 0000000..b7f89a4
--- /dev/null
+++ b/tests/qapi-schema/empty.out
@@ -0,0 +1,3 @@
+[]
+[]
+[]
diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/funny-char.exit b/tests/qapi-schema/funny-char.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/funny-char.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/funny-char.json b/tests/qapi-schema/funny-char.json
new file mode 100644
index 0000000..d4973a2
--- /dev/null
+++ b/tests/qapi-schema/funny-char.json
@@ -0,0 +1,2 @@
+{ 'enum': 'Status',
+ 'data': [ 'good', 'bad', 'ugly' ]; }
diff --git a/tests/qapi-schema/funny-char.out b/tests/qapi-schema/funny-char.out
new file mode 100644
index 0000000..e3bd904
--- /dev/null
+++ b/tests/qapi-schema/funny-char.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+['Status']
+[]
diff --git a/tests/qapi-schema/indented-expr.err b/tests/qapi-schema/indented-expr.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/indented-expr.exit b/tests/qapi-schema/indented-expr.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/indented-expr.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/indented-expr.json b/tests/qapi-schema/indented-expr.json
new file mode 100644
index 0000000..d80af60
--- /dev/null
+++ b/tests/qapi-schema/indented-expr.json
@@ -0,0 +1,2 @@
+{ 'id' : 'eins' }
+ { 'id' : 'zwei' }
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
new file mode 100644
index 0000000..98ae692
--- /dev/null
+++ b/tests/qapi-schema/indented-expr.out
@@ -0,0 +1,3 @@
+[OrderedDict([('id', 'eins')])]
+[]
+[]
diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/missing-colon.exit b/tests/qapi-schema/missing-colon.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/missing-colon.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/missing-colon.json b/tests/qapi-schema/missing-colon.json
new file mode 100644
index 0000000..6fc27ce
--- /dev/null
+++ b/tests/qapi-schema/missing-colon.json
@@ -0,0 +1,2 @@
+{ 'enum' 'Status',
+ 'data': [ 'good', 'bad', 'ugly' ] }
diff --git a/tests/qapi-schema/missing-colon.out b/tests/qapi-schema/missing-colon.out
new file mode 100644
index 0000000..50f827e
--- /dev/null
+++ b/tests/qapi-schema/missing-colon.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', ','), ('data', ['good', 'bad', 'ugly'])])]
+[',']
+[]
diff --git a/tests/qapi-schema/missing-comma.err b/tests/qapi-schema/missing-comma.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/missing-comma.exit b/tests/qapi-schema/missing-comma.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/missing-comma.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/missing-comma.json b/tests/qapi-schema/missing-comma.json
new file mode 100644
index 0000000..50f5178
--- /dev/null
+++ b/tests/qapi-schema/missing-comma.json
@@ -0,0 +1,2 @@
+{ 'enum': 'Status'
+ 'data': [ 'good', 'bad', 'ugly' ] }
diff --git a/tests/qapi-schema/missing-comma.out b/tests/qapi-schema/missing-comma.out
new file mode 100644
index 0000000..e3bd904
--- /dev/null
+++ b/tests/qapi-schema/missing-comma.out
@@ -0,0 +1,3 @@
+[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
+['Status']
+[]
diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err
new file mode 100644
index 0000000..48c849d
--- /dev/null
+++ b/tests/qapi-schema/non-objects.err
@@ -0,0 +1 @@
+Crashed: <type 'exceptions.AttributeError'>
diff --git a/tests/qapi-schema/non-objects.exit b/tests/qapi-schema/non-objects.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/non-objects.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/non-objects.json b/tests/qapi-schema/non-objects.json
new file mode 100644
index 0000000..f3fa851
--- /dev/null
+++ b/tests/qapi-schema/non-objects.json
@@ -0,0 +1,2 @@
+'string'
+[ ]
diff --git a/tests/qapi-schema/non-objects.out b/tests/qapi-schema/non-objects.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/quoted-structural-chars.exit b/tests/qapi-schema/quoted-structural-chars.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/quoted-structural-chars.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/quoted-structural-chars.json b/tests/qapi-schema/quoted-structural-chars.json
new file mode 100644
index 0000000..9fe657a
--- /dev/null
+++ b/tests/qapi-schema/quoted-structural-chars.json
@@ -0,0 +1 @@
+'{' 'key1' ':' 'value1' ',' 'key2' ':' '[' ']' '}'
diff --git a/tests/qapi-schema/quoted-structural-chars.out b/tests/qapi-schema/quoted-structural-chars.out
new file mode 100644
index 0000000..85405be
--- /dev/null
+++ b/tests/qapi-schema/quoted-structural-chars.out
@@ -0,0 +1,3 @@
+[OrderedDict([('key1', 'value1'), ('key2', [])])]
+[]
+[]
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
new file mode 100644
index 0000000..3280eff
--- /dev/null
+++ b/tests/qapi-schema/test-qapi.py
@@ -0,0 +1,25 @@
+#
+# QAPI parser test harness
+#
+# Copyright (c) 2013 Red Hat Inc.
+#
+# Authors:
+# Markus Armbruster <armbru@redhat.com>
+#
+# This work is licensed under the terms of the GNU GPL, version 2 or later.
+# See the COPYING file in the top-level directory.
+#
+
+from qapi import *
+from pprint import pprint
+import sys
+
+try:
+ exprs = parse_schema(sys.stdin)
+except:
+ print >>sys.stderr, "Crashed:", sys.exc_info()[0]
+ exit(1)
+
+pprint(exprs)
+pprint(enum_types)
+pprint(struct_types)
diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err
new file mode 100644
index 0000000..f9a9c2a
--- /dev/null
+++ b/tests/qapi-schema/unclosed-object.err
@@ -0,0 +1 @@
+Crashed: <type 'exceptions.IndexError'>
diff --git a/tests/qapi-schema/unclosed-object.exit b/tests/qapi-schema/unclosed-object.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/unclosed-object.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/unclosed-object.json b/tests/qapi-schema/unclosed-object.json
new file mode 100644
index 0000000..97673d5
--- /dev/null
+++ b/tests/qapi-schema/unclosed-object.json
@@ -0,0 +1 @@
+{ 'key': [ 'value'
diff --git a/tests/qapi-schema/unclosed-object.out b/tests/qapi-schema/unclosed-object.out
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/unclosed-string.err b/tests/qapi-schema/unclosed-string.err
new file mode 100644
index 0000000..5af46c2
--- /dev/null
+++ b/tests/qapi-schema/unclosed-string.err
@@ -0,0 +1 @@
+Crashed: <type 'exceptions.Exception'>
diff --git a/tests/qapi-schema/unclosed-string.exit b/tests/qapi-schema/unclosed-string.exit
new file mode 100644
index 0000000..d00491f
--- /dev/null
+++ b/tests/qapi-schema/unclosed-string.exit
@@ -0,0 +1 @@
+1
diff --git a/tests/qapi-schema/unclosed-string.json b/tests/qapi-schema/unclosed-string.json
new file mode 100644
index 0000000..8c16b6b
--- /dev/null
+++ b/tests/qapi-schema/unclosed-string.json
@@ -0,0 +1,2 @@
+{ 'text': 'lorem ips
+}
diff --git a/tests/qapi-schema/unclosed-string.out b/tests/qapi-schema/unclosed-string.out
new file mode 100644
index 0000000..e69de29
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests
2013-07-26 12:39 ` [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests Markus Armbruster
@ 2013-07-26 12:48 ` Eric Blake
2013-07-26 14:16 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-07-26 12:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 656 bytes --]
On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> The parser handles erroneous input badly. To be improved shortly.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
Lots of proof on how bad it is! I'd also like to see a couple tests on
trailing commas:
{ 'enum': 'Foo', [ 'bar' ], }
{ 'enum': 'Gur', [ 'ble', ] }
since we have had patches in the past to clean them up (shame on JSON
for copying C89 instead of C99 with regards to trailing commas).
Either way,
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests
2013-07-26 12:48 ` Eric Blake
@ 2013-07-26 14:16 ` Markus Armbruster
2013-07-26 14:57 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 14:16 UTC (permalink / raw)
To: Eric Blake; +Cc: aliguori, akong, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>> The parser handles erroneous input badly. To be improved shortly.
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>
> Lots of proof on how bad it is! I'd also like to see a couple tests on
> trailing commas:
>
> { 'enum': 'Foo', [ 'bar' ], }
> { 'enum': 'Gur', [ 'ble', ] }
I figure you mean
{ 'enum': 'Foo', 'data': [ 'bar' ], }
{ 'enum': 'Gur', 'data': [ 'ble', ] }
My parser rejects both:
<stdin>:1:37: Expected string
<stdin>:2:35: Expected "{", "[" or string
I commented out the first to get the second error. Making the parser
continue after errors didn't seem to be worthwhile.
> since we have had patches in the past to clean them up (shame on JSON
> for copying C89 instead of C99 with regards to trailing commas).
Indeed.
> Either way,
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests
2013-07-26 14:16 ` Markus Armbruster
@ 2013-07-26 14:57 ` Eric Blake
2013-07-26 15:31 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-07-26 14:57 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1594 bytes --]
On 07/26/2013 08:16 AM, Markus Armbruster wrote:
> Eric Blake <eblake@redhat.com> writes:
>
>> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>>> The parser handles erroneous input badly. To be improved shortly.
>>>
>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>> ---
>>
>> Lots of proof on how bad it is! I'd also like to see a couple tests on
>> trailing commas:
>>
>> { 'enum': 'Foo', [ 'bar' ], }
>> { 'enum': 'Gur', [ 'ble', ] }
>
> I figure you mean
>
> { 'enum': 'Foo', 'data': [ 'bar' ], }
> { 'enum': 'Gur', 'data': [ 'ble', ] }
Yep, you got my intent, even if I didn't type it right.
>
> My parser rejects both:
>
> <stdin>:1:37: Expected string
> <stdin>:2:35: Expected "{", "[" or string
Good!
>
> I commented out the first to get the second error. Making the parser
> continue after errors didn't seem to be worthwhile.
Agree about not continuing after errors; once the file is clean, anyone
adding a new command will have errors in at most the one command they
are trying to add, and even if it is an iterative approach to get them
to find all the problems, it's a lot easier to have that one developer
fix their work than trying to bake error recovery into the parser.
If you do add these two test cases (which I recommend), it should indeed
be two separate tests.
Another thought - is it worth testing other valid JSON but invalid
schema constructs, such as:
{ 'enum': 1, 'data': false }
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests
2013-07-26 14:57 ` Eric Blake
@ 2013-07-26 15:31 ` Markus Armbruster
0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 15:31 UTC (permalink / raw)
To: Eric Blake; +Cc: aliguori, akong, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 07/26/2013 08:16 AM, Markus Armbruster wrote:
>> Eric Blake <eblake@redhat.com> writes:
>>
>>> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>>>> The parser handles erroneous input badly. To be improved shortly.
>>>>
>>>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>>>> ---
>>>
>>> Lots of proof on how bad it is! I'd also like to see a couple tests on
>>> trailing commas:
>>>
>>> { 'enum': 'Foo', [ 'bar' ], }
>>> { 'enum': 'Gur', [ 'ble', ] }
>>
>> I figure you mean
>>
>> { 'enum': 'Foo', 'data': [ 'bar' ], }
>> { 'enum': 'Gur', 'data': [ 'ble', ] }
>
> Yep, you got my intent, even if I didn't type it right.
>
>>
>> My parser rejects both:
>>
>> <stdin>:1:37: Expected string
>> <stdin>:2:35: Expected "{", "[" or string
>
> Good!
>
>>
>> I commented out the first to get the second error. Making the parser
>> continue after errors didn't seem to be worthwhile.
>
> Agree about not continuing after errors; once the file is clean, anyone
> adding a new command will have errors in at most the one command they
> are trying to add, and even if it is an iterative approach to get them
> to find all the problems, it's a lot easier to have that one developer
> fix their work than trying to bake error recovery into the parser.
>
> If you do add these two test cases (which I recommend), it should indeed
> be two separate tests.
Could be done on top. If I need to respin anyway, I'll add them in the
first patch, of course.
> Another thought - is it worth testing other valid JSON but invalid
> schema constructs, such as:
>
> { 'enum': 1, 'data': false }
Maybe, but I limited myself just to the parser in this series.
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
2013-07-26 12:39 ` [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 13:17 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser Markus Armbruster
` (7 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi-schema-test.json | 53 ---------------------------------
tests/Makefile | 8 ++---
tests/qapi-schema/qapi-schema-test.exit | 1 +
tests/qapi-schema/qapi-schema-test.json | 53 +++++++++++++++++++++++++++++++++
tests/qapi-schema/qapi-schema-test.out | 19 ++++++++++++
5 files changed, 77 insertions(+), 57 deletions(-)
delete mode 100644 qapi-schema-test.json
create mode 100644 tests/qapi-schema/qapi-schema-test.err
create mode 100644 tests/qapi-schema/qapi-schema-test.exit
create mode 100644 tests/qapi-schema/qapi-schema-test.json
create mode 100644 tests/qapi-schema/qapi-schema-test.out
diff --git a/qapi-schema-test.json b/qapi-schema-test.json
deleted file mode 100644
index 4434fa3..0000000
--- a/qapi-schema-test.json
+++ /dev/null
@@ -1,53 +0,0 @@
-# *-*- Mode: Python -*-*
-
-# for testing enums
-{ 'enum': 'EnumOne',
- 'data': [ 'value1', 'value2', 'value3' ] }
-{ 'type': 'NestedEnumsOne',
- 'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
-
-# for testing nested structs
-{ 'type': 'UserDefOne',
- 'data': { 'integer': 'int', 'string': 'str', '*enum1': 'EnumOne' } }
-
-{ 'type': 'UserDefTwo',
- 'data': { 'string': 'str',
- 'dict': { 'string': 'str',
- 'dict': { 'userdef': 'UserDefOne', 'string': 'str' },
- '*dict2': { 'userdef': 'UserDefOne', 'string': 'str' } } } }
-
-{ 'type': 'UserDefNested',
- 'data': { 'string0': 'str',
- 'dict1': { 'string1': 'str',
- 'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' },
- '*dict3': { 'userdef2': 'UserDefOne', 'string3': 'str' } } } }
-
-# for testing unions
-{ 'type': 'UserDefA',
- 'data': { 'boolean': 'bool' } }
-
-{ 'type': 'UserDefB',
- 'data': { 'integer': 'int' } }
-
-{ 'union': 'UserDefUnion',
- 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
-
-# for testing native lists
-{ 'union': 'UserDefNativeListUnion',
- 'data': { 'integer': ['int'],
- 's8': ['int8'],
- 's16': ['int16'],
- 's32': ['int32'],
- 's64': ['int64'],
- 'u8': ['uint8'],
- 'u16': ['uint16'],
- 'u32': ['uint32'],
- 'u64': ['uint64'],
- 'number': ['number'],
- 'boolean': ['bool'],
- 'string': ['str'] } }
-
-# testing commands
-{ 'command': 'user_def_cmd', 'data': {} }
-{ 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
-{ 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
diff --git a/tests/Makefile b/tests/Makefile
index 89a467b..4038b29 100644
--- a/tests/Makefile
+++ b/tests/Makefile
@@ -85,7 +85,7 @@ check-qtest-ppc64-y += tests/boot-order-test$(EXESUF)
check-qapi-schema-y := $(addprefix tests/qapi-schema/, empty.json \
funny-char.json indented-expr.json missing-colon.json \
- missing-comma.json non-objects.json \
+ missing-comma.json non-objects.json qapi-schema-test.json \
quoted-structural-chars.json unclosed-object.json \
unclosed-string.json)
@@ -123,13 +123,13 @@ tests/test-cutils$(EXESUF): tests/test-cutils.o util/cutils.o
tests/test-int128$(EXESUF): tests/test-int128.o
tests/test-qapi-types.c tests/test-qapi-types.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-types.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-types.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qapi-visit.c tests/test-qapi-visit.h :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-visit.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-visit.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-qmp-commands.h tests/test-qmp-marshal.c :\
-$(SRC_PATH)/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
+$(SRC_PATH)/tests/qapi-schema/qapi-schema-test.json $(SRC_PATH)/scripts/qapi-commands.py
$(call quiet-command,$(PYTHON) $(SRC_PATH)/scripts/qapi-commands.py $(gen-out-type) -o tests -p "test-" < $<, " GEN $@")
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y) libqemuutil.a libqemustub.a
diff --git a/tests/qapi-schema/qapi-schema-test.err b/tests/qapi-schema/qapi-schema-test.err
new file mode 100644
index 0000000..e69de29
diff --git a/tests/qapi-schema/qapi-schema-test.exit b/tests/qapi-schema/qapi-schema-test.exit
new file mode 100644
index 0000000..573541a
--- /dev/null
+++ b/tests/qapi-schema/qapi-schema-test.exit
@@ -0,0 +1 @@
+0
diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
new file mode 100644
index 0000000..4434fa3
--- /dev/null
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -0,0 +1,53 @@
+# *-*- Mode: Python -*-*
+
+# for testing enums
+{ 'enum': 'EnumOne',
+ 'data': [ 'value1', 'value2', 'value3' ] }
+{ 'type': 'NestedEnumsOne',
+ 'data': { 'enum1': 'EnumOne', '*enum2': 'EnumOne', 'enum3': 'EnumOne', '*enum4': 'EnumOne' } }
+
+# for testing nested structs
+{ 'type': 'UserDefOne',
+ 'data': { 'integer': 'int', 'string': 'str', '*enum1': 'EnumOne' } }
+
+{ 'type': 'UserDefTwo',
+ 'data': { 'string': 'str',
+ 'dict': { 'string': 'str',
+ 'dict': { 'userdef': 'UserDefOne', 'string': 'str' },
+ '*dict2': { 'userdef': 'UserDefOne', 'string': 'str' } } } }
+
+{ 'type': 'UserDefNested',
+ 'data': { 'string0': 'str',
+ 'dict1': { 'string1': 'str',
+ 'dict2': { 'userdef1': 'UserDefOne', 'string2': 'str' },
+ '*dict3': { 'userdef2': 'UserDefOne', 'string3': 'str' } } } }
+
+# for testing unions
+{ 'type': 'UserDefA',
+ 'data': { 'boolean': 'bool' } }
+
+{ 'type': 'UserDefB',
+ 'data': { 'integer': 'int' } }
+
+{ 'union': 'UserDefUnion',
+ 'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
+
+# for testing native lists
+{ 'union': 'UserDefNativeListUnion',
+ 'data': { 'integer': ['int'],
+ 's8': ['int8'],
+ 's16': ['int16'],
+ 's32': ['int32'],
+ 's64': ['int64'],
+ 'u8': ['uint8'],
+ 'u16': ['uint16'],
+ 'u32': ['uint32'],
+ 'u64': ['uint64'],
+ 'number': ['number'],
+ 'boolean': ['bool'],
+ 'string': ['str'] } }
+
+# testing commands
+{ 'command': 'user_def_cmd', 'data': {} }
+{ 'command': 'user_def_cmd1', 'data': {'ud1a': 'UserDefOne'} }
+{ 'command': 'user_def_cmd2', 'data': {'ud1a': 'UserDefOne', 'ud1b': 'UserDefOne'}, 'returns': 'UserDefTwo' }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
new file mode 100644
index 0000000..fb00344
--- /dev/null
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -0,0 +1,19 @@
+[OrderedDict([('enum', 'EnumOne'), ('data', ['value1', 'value2', 'value3'])]),
+ OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
+ OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('union', 'UserDefUnion'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
+ OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
+ OrderedDict([('command', 'user_def_cmd1'), ('data', OrderedDict([('ud1a', 'UserDefOne')]))]),
+ OrderedDict([('command', 'user_def_cmd2'), ('data', OrderedDict([('ud1a', 'UserDefOne'), ('ud1b', 'UserDefOne')])), ('returns', 'UserDefTwo')])]
+['EnumOne', 'UserDefUnionKind', 'UserDefNativeListUnionKind']
+[OrderedDict([('type', 'NestedEnumsOne'), ('data', OrderedDict([('enum1', 'EnumOne'), ('*enum2', 'EnumOne'), ('enum3', 'EnumOne'), ('*enum4', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefOne'), ('data', OrderedDict([('integer', 'int'), ('string', 'str'), ('*enum1', 'EnumOne')]))]),
+ OrderedDict([('type', 'UserDefTwo'), ('data', OrderedDict([('string', 'str'), ('dict', OrderedDict([('string', 'str'), ('dict', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')])), ('*dict2', OrderedDict([('userdef', 'UserDefOne'), ('string', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
+ OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
+ OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))])]
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test
2013-07-26 12:39 ` [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test Markus Armbruster
@ 2013-07-26 13:17 ` Eric Blake
2013-07-27 15:34 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-07-26 13:17 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1183 bytes --]
On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> qapi-schema-test.json | 53 ---------------------------------
> tests/Makefile | 8 ++---
> tests/qapi-schema/qapi-schema-test.exit | 1 +
> tests/qapi-schema/qapi-schema-test.json | 53 +++++++++++++++++++++++++++++++++
You don't have git rename detection turned on? You get a more compact
listing of file renames if you do:
git config diff.renames true
I just modified http://wiki.qemu.org/Contribute/SubmitAPatch to mention
this tip.
> tests/qapi-schema/qapi-schema-test.out | 19 ++++++++++++
> 5 files changed, 77 insertions(+), 57 deletions(-)
> delete mode 100644 qapi-schema-test.json
> create mode 100644 tests/qapi-schema/qapi-schema-test.err
> create mode 100644 tests/qapi-schema/qapi-schema-test.exit
> create mode 100644 tests/qapi-schema/qapi-schema-test.json
> create mode 100644 tests/qapi-schema/qapi-schema-test.out
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test
2013-07-26 13:17 ` Eric Blake
@ 2013-07-27 15:34 ` Markus Armbruster
0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2013-07-27 15:34 UTC (permalink / raw)
To: Eric Blake; +Cc: aliguori, akong, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> qapi-schema-test.json | 53 ---------------------------------
>> tests/Makefile | 8 ++---
>> tests/qapi-schema/qapi-schema-test.exit | 1 +
>> tests/qapi-schema/qapi-schema-test.json | 53
>> +++++++++++++++++++++++++++++++++
>
> You don't have git rename detection turned on? You get a more compact
> listing of file renames if you do:
> git config diff.renames true
Now I do.
> I just modified http://wiki.qemu.org/Contribute/SubmitAPatch to mention
> this tip.
Good move.
>> tests/qapi-schema/qapi-schema-test.out | 19 ++++++++++++
>> 5 files changed, 77 insertions(+), 57 deletions(-)
>> delete mode 100644 qapi-schema-test.json
>> create mode 100644 tests/qapi-schema/qapi-schema-test.err
>> create mode 100644 tests/qapi-schema/qapi-schema-test.exit
>> create mode 100644 tests/qapi-schema/qapi-schema-test.json
>> create mode 100644 tests/qapi-schema/qapi-schema-test.out
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
Thanks!
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
2013-07-26 12:39 ` [Qemu-devel] [PATCH 1/9] tests: QAPI schema parser tests Markus Armbruster
2013-07-26 12:39 ` [Qemu-devel] [PATCH 2/9] tests: Use qapi-schema-test.json as schema parser test Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 13:54 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting Markus Armbruster
` (6 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
The parser has a rather unorthodox structure:
Until EOF:
Read a section:
Generator function get_expr() yields one section after the
other, as a string. An unindented, non-empty line that
isn't a comment starts a new section.
Lexing:
Split section into a list of tokens (strings), with help
of generator function tokenize().
Parsing:
Parse the first expression from the list of tokens, with
parse(), throw away any remaining tokens.
In parse_schema(): record value of an enum, union or
struct key (if any) in the appropriate global table,
append expression to the list of expressions.
Return list of expressions.
Known issues:
(1) Indentation is significant, unlike in real JSON.
(2) Neither lexer nor parser have any idea of source positions. Error
reporting is hard, let's go shopping.
(3) The one error we bother to detect, we "report" via raise.
(4) The lexer silently ignores invalid characters.
(5) If everything in a section gets ignored, the parser crashes.
(6) The lexer treats a string containing a structural character exactly
like the structural character.
(7) Tokens trailing the first expression in a section are silently
ignored.
(8) The parser accepts any token in place of a colon.
(9) The parser treats comma as optional.
(10) parse() crashes on unexpected EOF.
(11) parse_schema() crashes when a section's expression isn't a JSON
object.
Replace this piece of original art by a thoroughly unoriginal design.
Takes care of (1), (2), (5), (6) and (7), and lays the groundwork for
addressing the others. Generated source files remain unchanged.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 163 +++++++++++++------------
tests/qapi-schema/indented-expr.out | 2 +-
tests/qapi-schema/missing-colon.out | 4 +-
tests/qapi-schema/quoted-structural-chars.err | 1 +
tests/qapi-schema/quoted-structural-chars.exit | 2 +-
tests/qapi-schema/quoted-structural-chars.out | 3 -
6 files changed, 88 insertions(+), 87 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index baf1321..cac56b7 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -2,9 +2,11 @@
# QAPI helper library
#
# Copyright IBM, Corp. 2011
+# Copyright (c) 2013 Red Hat Inc.
#
# Authors:
# Anthony Liguori <aliguori@us.ibm.com>
+# Markus Armbruster <armbru@redhat.com>
#
# This work is licensed under the terms of the GNU GPLv2.
# See the COPYING.LIB file in the top-level directory.
@@ -17,91 +19,92 @@ builtin_types = [
'uint8', 'uint16', 'uint32', 'uint64'
]
-def tokenize(data):
- while len(data):
- ch = data[0]
- data = data[1:]
- if ch in ['{', '}', ':', ',', '[', ']']:
- yield ch
- elif ch in ' \n':
- None
- elif ch == "'":
- string = ''
- esc = False
- while True:
- if (data == ''):
- raise Exception("Mismatched quotes")
- ch = data[0]
- data = data[1:]
- if esc:
- string += ch
- esc = False
- elif ch == "\\":
- esc = True
- elif ch == "'":
- break
- else:
- string += ch
- yield string
-
-def parse(tokens):
- if tokens[0] == '{':
- ret = OrderedDict()
- tokens = tokens[1:]
- while tokens[0] != '}':
- key = tokens[0]
- tokens = tokens[1:]
-
- tokens = tokens[1:] # :
-
- value, tokens = parse(tokens)
-
- if tokens[0] == ',':
- tokens = tokens[1:]
-
- ret[key] = value
- tokens = tokens[1:]
- return ret, tokens
- elif tokens[0] == '[':
- ret = []
- tokens = tokens[1:]
- while tokens[0] != ']':
- value, tokens = parse(tokens)
- if tokens[0] == ',':
- tokens = tokens[1:]
- ret.append(value)
- tokens = tokens[1:]
- return ret, tokens
- else:
- return tokens[0], tokens[1:]
-
-def evaluate(string):
- return parse(map(lambda x: x, tokenize(string)))[0]
-
-def get_expr(fp):
- expr = ''
-
- for line in fp:
- if line.startswith('#') or line == '\n':
- continue
-
- if line.startswith(' '):
- expr += line
- elif expr:
- yield expr
- expr = line
+class QAPISchema:
+
+ def __init__(self, fp):
+ self.fp = fp
+ self.src = fp.read()
+ if self.src == '' or self.src[-1] != '\n':
+ self.src += '\n'
+ self.cursor = 0
+ self.exprs = []
+ self.accept()
+
+ while self.tok != None:
+ self.exprs.append(self.get_expr())
+
+ def accept(self):
+ while True:
+ bol = self.cursor == 0 or self.src[self.cursor-1] == '\n'
+ self.tok = self.src[self.cursor]
+ self.cursor += 1
+ self.val = None
+
+ if self.tok == '#' and bol:
+ self.cursor = self.src.find('\n', self.cursor)
+ elif self.tok in ['{', '}', ':', ',', '[', ']']:
+ return
+ elif self.tok == "'":
+ string = ''
+ esc = False
+ while True:
+ ch = self.src[self.cursor]
+ self.cursor += 1
+ if ch == '\n':
+ raise Exception("Mismatched quotes")
+ if esc:
+ string += ch
+ esc = False
+ elif ch == "\\":
+ esc = True
+ elif ch == "'":
+ self.val = string
+ return
+ else:
+ string += ch
+ elif self.tok == '\n':
+ if self.cursor == len(self.src):
+ self.tok = None
+ return
+
+ def get_members(self):
+ expr = OrderedDict()
+ while self.tok != '}':
+ key = self.val
+ self.accept()
+ self.accept() # :
+ expr[key] = self.get_expr()
+ if self.tok == ',':
+ self.accept()
+ self.accept()
+ return expr
+
+ def get_values(self):
+ expr = []
+ while self.tok != ']':
+ expr.append(self.get_expr())
+ if self.tok == ',':
+ self.accept()
+ self.accept()
+ return expr
+
+ def get_expr(self):
+ if self.tok == '{':
+ self.accept()
+ expr = self.get_members()
+ elif self.tok == '[':
+ self.accept()
+ expr = self.get_values()
else:
- expr += line
-
- if expr:
- yield expr
+ expr = self.val
+ self.accept()
+ return expr
def parse_schema(fp):
+ schema = QAPISchema(fp)
exprs = []
- for expr in get_expr(fp):
- expr_eval = evaluate(expr)
-
+ for expr_eval in schema.exprs:
if expr_eval.has_key('enum'):
add_enum(expr_eval['enum'])
elif expr_eval.has_key('union'):
diff --git a/tests/qapi-schema/indented-expr.out b/tests/qapi-schema/indented-expr.out
index 98ae692..98af89a 100644
--- a/tests/qapi-schema/indented-expr.out
+++ b/tests/qapi-schema/indented-expr.out
@@ -1,3 +1,3 @@
-[OrderedDict([('id', 'eins')])]
+[OrderedDict([('id', 'eins')]), OrderedDict([('id', 'zwei')])]
[]
[]
diff --git a/tests/qapi-schema/missing-colon.out b/tests/qapi-schema/missing-colon.out
index 50f827e..e67068c 100644
--- a/tests/qapi-schema/missing-colon.out
+++ b/tests/qapi-schema/missing-colon.out
@@ -1,3 +1,3 @@
-[OrderedDict([('enum', ','), ('data', ['good', 'bad', 'ugly'])])]
-[',']
+[OrderedDict([('enum', None), ('data', ['good', 'bad', 'ugly'])])]
+[None]
[]
diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err
index e69de29..48c849d 100644
--- a/tests/qapi-schema/quoted-structural-chars.err
+++ b/tests/qapi-schema/quoted-structural-chars.err
@@ -0,0 +1 @@
+Crashed: <type 'exceptions.AttributeError'>
diff --git a/tests/qapi-schema/quoted-structural-chars.exit b/tests/qapi-schema/quoted-structural-chars.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/quoted-structural-chars.exit
+++ b/tests/qapi-schema/quoted-structural-chars.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/quoted-structural-chars.out b/tests/qapi-schema/quoted-structural-chars.out
index 85405be..e69de29 100644
--- a/tests/qapi-schema/quoted-structural-chars.out
+++ b/tests/qapi-schema/quoted-structural-chars.out
@@ -1,3 +0,0 @@
-[OrderedDict([('key1', 'value1'), ('key2', [])])]
-[]
-[]
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser
2013-07-26 12:39 ` [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser Markus Armbruster
@ 2013-07-26 13:54 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-07-26 13:54 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1930 bytes --]
On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> The parser has a rather unorthodox structure:
>
>
> Replace this piece of original art by a thoroughly unoriginal design.
I sense a bit of tongue-in-cheek urge to use more colorful language at
this point, describing the horrors of the old implementation. Thank you
for thinking of the children :)
> Takes care of (1), (2), (5), (6) and (7), and lays the groundwork for
> addressing the others. Generated source files remain unchanged.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi.py | 163 +++++++++++++------------
> tests/qapi-schema/indented-expr.out | 2 +-
> tests/qapi-schema/missing-colon.out | 4 +-
> tests/qapi-schema/quoted-structural-chars.err | 1 +
> tests/qapi-schema/quoted-structural-chars.exit | 2 +-
> tests/qapi-schema/quoted-structural-chars.out | 3 -
> 6 files changed, 88 insertions(+), 87 deletions(-)
My python review skills are weak, so take this review with a grain of salt.
> +
> + def get_members(self):
> + expr = OrderedDict()
> + while self.tok != '}':
> + key = self.val
> + self.accept()
> + self.accept() # :
> + expr[key] = self.get_expr()
> + if self.tok == ',':
> + self.accept()
My comments in 1/9 about a test of trailing comma behavior may impact
this code.
> + self.accept()
> + return expr
> +
> + def get_values(self):
> + expr = []
> + while self.tok != ']':
> + expr.append(self.get_expr())
> + if self.tok == ',':
> + self.accept()
and this code.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (2 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 3/9] qapi.py: Restructure lexer and parser Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 15:30 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 5/9] qapi.py: Reject invalid characters in schema file Markus Armbruster
` (5 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 29 +++++++++++++++++++++++++++--
tests/qapi-schema/test-qapi.py | 2 ++
tests/qapi-schema/unclosed-string.err | 2 +-
3 files changed, 30 insertions(+), 3 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index cac56b7..35ff76e 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -12,6 +12,7 @@
# See the COPYING.LIB file in the top-level directory.
from ordereddict import OrderedDict
+import sys
builtin_types = [
'str', 'int', 'number', 'bool',
@@ -19,6 +20,23 @@ builtin_types = [
'uint8', 'uint16', 'uint32', 'uint64'
]
+class QAPISchemaError(Exception):
+ def __init__(self, schema, msg):
+ self.fp = schema.fp
+ self.msg = msg
+ self.line = self.col = 1
+ for ch in schema.src[0:schema.pos]:
+ if ch == '\n':
+ self.line += 1
+ self.col = 1
+ elif ch == '\t':
+ self.col = (self.col + 7) % 8 + 1
+ else:
+ self.col += 1
+
+ def __str__(self):
+ return "%s:%s:%s: %s" % (self.fp.name, self.line, self.col, self.msg)
+
class QAPISchema:
def __init__(self, fp):
@@ -37,6 +55,7 @@ class QAPISchema:
while True:
bol = self.cursor == 0 or self.src[self.cursor-1] == '\n'
self.tok = self.src[self.cursor]
+ self.pos = self.cursor
self.cursor += 1
self.val = None
@@ -51,7 +70,8 @@ class QAPISchema:
ch = self.src[self.cursor]
self.cursor += 1
if ch == '\n':
- raise Exception("Mismatched quotes")
+ raise QAPISchemaError(self,
+ 'Missing terminating "\'"')
if esc:
string += ch
esc = False
@@ -101,7 +121,12 @@ class QAPISchema:
return expr
def parse_schema(fp):
- schema = QAPISchema(fp)
+ try:
+ schema = QAPISchema(fp)
+ except QAPISchemaError as e:
+ print >>sys.stderr, e
+ exit(1)
+
exprs = []
for expr_eval in schema.exprs:
diff --git a/tests/qapi-schema/test-qapi.py b/tests/qapi-schema/test-qapi.py
index 3280eff..b3d1e1d 100644
--- a/tests/qapi-schema/test-qapi.py
+++ b/tests/qapi-schema/test-qapi.py
@@ -16,6 +16,8 @@ import sys
try:
exprs = parse_schema(sys.stdin)
+except SystemExit:
+ raise
except:
print >>sys.stderr, "Crashed:", sys.exc_info()[0]
exit(1)
diff --git a/tests/qapi-schema/unclosed-string.err b/tests/qapi-schema/unclosed-string.err
index 5af46c2..948d883 100644
--- a/tests/qapi-schema/unclosed-string.err
+++ b/tests/qapi-schema/unclosed-string.err
@@ -1 +1 @@
-Crashed: <type 'exceptions.Exception'>
+<stdin>:1:11: Missing terminating "'"
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting
2013-07-26 12:39 ` [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting Markus Armbruster
@ 2013-07-26 15:30 ` Eric Blake
2013-07-26 19:33 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-07-26 15:30 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1213 bytes --]
On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi.py | 29 +++++++++++++++++++++++++++--
> tests/qapi-schema/test-qapi.py | 2 ++
> tests/qapi-schema/unclosed-string.err | 2 +-
> 3 files changed, 30 insertions(+), 3 deletions(-)
>
> +class QAPISchemaError(Exception):
> + def __init__(self, schema, msg):
> + self.fp = schema.fp
> + self.msg = msg
> + self.line = self.col = 1
> + for ch in schema.src[0:schema.pos]:
> + if ch == '\n':
> + self.line += 1
> + self.col = 1
> + elif ch == '\t':
> + self.col = (self.col + 7) % 8 + 1
Do we even want to allow TABs in the schema files? Right now, they are
tab-free; if you error out here instead of futzing with tab width, we
could forcefully maintain that property.
> + else:
> + self.col += 1
Does python support ++ as shorthand for += 1?
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting
2013-07-26 15:30 ` Eric Blake
@ 2013-07-26 19:33 ` Markus Armbruster
2013-07-26 19:48 ` Paolo Bonzini
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 19:33 UTC (permalink / raw)
To: Eric Blake; +Cc: aliguori, akong, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> scripts/qapi.py | 29 +++++++++++++++++++++++++++--
>> tests/qapi-schema/test-qapi.py | 2 ++
>> tests/qapi-schema/unclosed-string.err | 2 +-
>> 3 files changed, 30 insertions(+), 3 deletions(-)
>
>>
>> +class QAPISchemaError(Exception):
>> + def __init__(self, schema, msg):
>> + self.fp = schema.fp
>> + self.msg = msg
>> + self.line = self.col = 1
>> + for ch in schema.src[0:schema.pos]:
>> + if ch == '\n':
>> + self.line += 1
>> + self.col = 1
>> + elif ch == '\t':
>> + self.col = (self.col + 7) % 8 + 1
>
> Do we even want to allow TABs in the schema files? Right now, they are
> tab-free; if you error out here instead of futzing with tab width, we
> could forcefully maintain that property.
I'm not volunteering for the TAB police, but if y'all want the parser to
reject TABs, I can do that.
>> + else:
>> + self.col += 1
>
> Does python support ++ as shorthand for += 1?
It doesn't, sadly.
> Reviewed-by: Eric Blake <eblake@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting
2013-07-26 19:33 ` Markus Armbruster
@ 2013-07-26 19:48 ` Paolo Bonzini
2013-07-26 19:57 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Paolo Bonzini @ 2013-07-26 19:48 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
Il 26/07/2013 21:33, Markus Armbruster ha scritto:
> > Do we even want to allow TABs in the schema files? Right now, they are
> > tab-free; if you error out here instead of futzing with tab width, we
> > could forcefully maintain that property.
>
> I'm not volunteering for the TAB police, but if y'all want the parser to
> reject TABs, I can do that.
No, please don't. TAB is whitespace in JSON, and should be whitespace
in schema files.
Paolo
^ permalink raw reply [flat|nested] 32+ messages in thread
* Re: [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting
2013-07-26 19:48 ` Paolo Bonzini
@ 2013-07-26 19:57 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-07-26 19:57 UTC (permalink / raw)
To: Paolo Bonzini; +Cc: mdroth, aliguori, akong, Markus Armbruster, qemu-devel
[-- Attachment #1: Type: text/plain, Size: 832 bytes --]
On 07/26/2013 01:48 PM, Paolo Bonzini wrote:
> Il 26/07/2013 21:33, Markus Armbruster ha scritto:
>>> Do we even want to allow TABs in the schema files? Right now, they are
>>> tab-free; if you error out here instead of futzing with tab width, we
>>> could forcefully maintain that property.
>>
>> I'm not volunteering for the TAB police, but if y'all want the parser to
>> reject TABs, I can do that.
>
> No, please don't. TAB is whitespace in JSON, and should be whitespace
> in schema files.
Good argument. Just because we don't use it now doesn't mean we have to
actively forbid it from future use; and the more we diverge from the
JSON spec, the more we are stuck maintaining our parser by hand.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 5/9] qapi.py: Reject invalid characters in schema file
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (3 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 4/9] qapi.py: Decent syntax error reporting Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 15:32 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically Markus Armbruster
` (4 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 2 ++
tests/qapi-schema/funny-char.err | 1 +
tests/qapi-schema/funny-char.exit | 2 +-
tests/qapi-schema/funny-char.out | 3 ---
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 35ff76e..f72a0e3 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -86,6 +86,8 @@ class QAPISchema:
if self.cursor == len(self.src):
self.tok = None
return
+ elif not self.tok.isspace():
+ raise QAPISchemaError(self, 'Stray "%s"' % self.tok)
def get_members(self):
expr = OrderedDict()
diff --git a/tests/qapi-schema/funny-char.err b/tests/qapi-schema/funny-char.err
index e69de29..d3dd293 100644
--- a/tests/qapi-schema/funny-char.err
+++ b/tests/qapi-schema/funny-char.err
@@ -0,0 +1 @@
+<stdin>:2:36: Stray ";"
diff --git a/tests/qapi-schema/funny-char.exit b/tests/qapi-schema/funny-char.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/funny-char.exit
+++ b/tests/qapi-schema/funny-char.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/funny-char.out b/tests/qapi-schema/funny-char.out
index e3bd904..e69de29 100644
--- a/tests/qapi-schema/funny-char.out
+++ b/tests/qapi-schema/funny-char.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
-[]
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (4 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 5/9] qapi.py: Reject invalid characters in schema file Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 15:56 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 7/9] qapi.py: Fix diagnosing non-objects at a schema's top-level Markus Armbruster
` (3 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Fixes at least the following parser bugs:
* accepts any token in place of a colon
* treats comma as optional
* crashes when closing braces or brackets are missing
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 40 ++++++++++++++++++++++++++---------
tests/qapi-schema/missing-colon.err | 1 +
tests/qapi-schema/missing-colon.exit | 2 +-
tests/qapi-schema/missing-colon.out | 3 ---
tests/qapi-schema/missing-comma.err | 1 +
tests/qapi-schema/missing-comma.exit | 2 +-
tests/qapi-schema/missing-comma.out | 3 ---
tests/qapi-schema/unclosed-object.err | 2 +-
8 files changed, 35 insertions(+), 19 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index f72a0e3..a7feccb 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -91,24 +91,42 @@ class QAPISchema:
def get_members(self):
expr = OrderedDict()
- while self.tok != '}':
+ if self.tok == '}':
+ self.accept()
+ return expr
+ if self.tok != "'":
+ raise QAPISchemaError(self, 'Expected string or "}"')
+ while True:
key = self.val
self.accept()
- self.accept() # :
+ if self.tok != ':':
+ raise QAPISchemaError(self, 'Expected ":"')
+ self.accept()
expr[key] = self.get_expr()
- if self.tok == ',':
+ if self.tok == '}':
self.accept()
- self.accept()
- return expr
+ return expr
+ if self.tok != ',':
+ raise QAPISchemaError(self, 'Expected "," or "}"')
+ self.accept()
+ if self.tok != "'":
+ raise QAPISchemaError(self, 'Expected string')
def get_values(self):
expr = []
- while self.tok != ']':
+ if self.tok == ']':
+ self.accept()
+ return expr
+ if not self.tok in [ '{', '[', "'" ]:
+ raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
+ while True:
expr.append(self.get_expr())
- if self.tok == ',':
+ if self.tok == ']':
self.accept()
- self.accept()
- return expr
+ return expr
+ if self.tok != ',':
+ raise QAPISchemaError(self, 'Expected "," or "]"')
+ self.accept()
def get_expr(self):
if self.tok == '{':
@@ -117,9 +135,11 @@ class QAPISchema:
elif self.tok == '[':
self.accept()
expr = self.get_values()
- else:
+ elif self.tok == "'":
expr = self.val
self.accept()
+ else:
+ raise QAPISchemaError(self, 'Expected "{", "[" or string')
return expr
def parse_schema(fp):
diff --git a/tests/qapi-schema/missing-colon.err b/tests/qapi-schema/missing-colon.err
index e69de29..9f2a355 100644
--- a/tests/qapi-schema/missing-colon.err
+++ b/tests/qapi-schema/missing-colon.err
@@ -0,0 +1 @@
+<stdin>:1:10: Expected ":"
diff --git a/tests/qapi-schema/missing-colon.exit b/tests/qapi-schema/missing-colon.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/missing-colon.exit
+++ b/tests/qapi-schema/missing-colon.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/missing-colon.out b/tests/qapi-schema/missing-colon.out
index e67068c..e69de29 100644
--- a/tests/qapi-schema/missing-colon.out
+++ b/tests/qapi-schema/missing-colon.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', None), ('data', ['good', 'bad', 'ugly'])])]
-[None]
-[]
diff --git a/tests/qapi-schema/missing-comma.err b/tests/qapi-schema/missing-comma.err
index e69de29..b0121b5 100644
--- a/tests/qapi-schema/missing-comma.err
+++ b/tests/qapi-schema/missing-comma.err
@@ -0,0 +1 @@
+<stdin>:2:3: Expected "," or "}"
diff --git a/tests/qapi-schema/missing-comma.exit b/tests/qapi-schema/missing-comma.exit
index 573541a..d00491f 100644
--- a/tests/qapi-schema/missing-comma.exit
+++ b/tests/qapi-schema/missing-comma.exit
@@ -1 +1 @@
-0
+1
diff --git a/tests/qapi-schema/missing-comma.out b/tests/qapi-schema/missing-comma.out
index e3bd904..e69de29 100644
--- a/tests/qapi-schema/missing-comma.out
+++ b/tests/qapi-schema/missing-comma.out
@@ -1,3 +0,0 @@
-[OrderedDict([('enum', 'Status'), ('data', ['good', 'bad', 'ugly'])])]
-['Status']
-[]
diff --git a/tests/qapi-schema/unclosed-object.err b/tests/qapi-schema/unclosed-object.err
index f9a9c2a..cc11626 100644
--- a/tests/qapi-schema/unclosed-object.err
+++ b/tests/qapi-schema/unclosed-object.err
@@ -1 +1 @@
-Crashed: <type 'exceptions.IndexError'>
+<stdin>:1:19: Expected "," or "]"
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically
2013-07-26 12:39 ` [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically Markus Armbruster
@ 2013-07-26 15:56 ` Eric Blake
2013-07-26 19:35 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Eric Blake @ 2013-07-26 15:56 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1440 bytes --]
On 07/26/2013 06:39 AM, Markus Armbruster wrote:
> Fixes at least the following parser bugs:
>
> * accepts any token in place of a colon
>
> * treats comma as optional
>
> * crashes when closing braces or brackets are missing
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
> scripts/qapi.py | 40 ++++++++++++++++++++++++++---------
> tests/qapi-schema/missing-colon.err | 1 +
> tests/qapi-schema/missing-colon.exit | 2 +-
> tests/qapi-schema/missing-colon.out | 3 ---
> tests/qapi-schema/missing-comma.err | 1 +
> tests/qapi-schema/missing-comma.exit | 2 +-
> tests/qapi-schema/missing-comma.out | 3 ---
> tests/qapi-schema/unclosed-object.err | 2 +-
> 8 files changed, 35 insertions(+), 19 deletions(-)
>
>
> def get_values(self):
> expr = []
> - while self.tok != ']':
> + if self.tok == ']':
> + self.accept()
> + return expr
> + if not self.tok in [ '{', '[', "'" ]:
> + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
JSON allows primitives here, as in [ 1 ]; but I agree that for the
purposes of our schema we will always be taking a string or complex
object whenever we have a list.
Reviewed-by: Eric Blake <eblake@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically
2013-07-26 15:56 ` Eric Blake
@ 2013-07-26 19:35 ` Markus Armbruster
2013-07-26 19:42 ` Eric Blake
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 19:35 UTC (permalink / raw)
To: Eric Blake; +Cc: aliguori, akong, qemu-devel, mdroth
Eric Blake <eblake@redhat.com> writes:
> On 07/26/2013 06:39 AM, Markus Armbruster wrote:
>> Fixes at least the following parser bugs:
>>
>> * accepts any token in place of a colon
>>
>> * treats comma as optional
>>
>> * crashes when closing braces or brackets are missing
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> ---
>> scripts/qapi.py | 40 ++++++++++++++++++++++++++---------
>> tests/qapi-schema/missing-colon.err | 1 +
>> tests/qapi-schema/missing-colon.exit | 2 +-
>> tests/qapi-schema/missing-colon.out | 3 ---
>> tests/qapi-schema/missing-comma.err | 1 +
>> tests/qapi-schema/missing-comma.exit | 2 +-
>> tests/qapi-schema/missing-comma.out | 3 ---
>> tests/qapi-schema/unclosed-object.err | 2 +-
>> 8 files changed, 35 insertions(+), 19 deletions(-)
>>
>
>>
>> def get_values(self):
>> expr = []
>> - while self.tok != ']':
>> + if self.tok == ']':
>> + self.accept()
>> + return expr
>> + if not self.tok in [ '{', '[', "'" ]:
>> + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
>
> JSON allows primitives here, as in [ 1 ]; but I agree that for the
> purposes of our schema we will always be taking a string or complex
> object whenever we have a list.
The lexer doesn't recognize any atoms but strings. If we change that,
the syntax error messages need to be reviewed (not just this one).
> Reviewed-by: Eric Blake <eblake@redhat.com>
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically
2013-07-26 19:35 ` Markus Armbruster
@ 2013-07-26 19:42 ` Eric Blake
0 siblings, 0 replies; 32+ messages in thread
From: Eric Blake @ 2013-07-26 19:42 UTC (permalink / raw)
To: Markus Armbruster; +Cc: aliguori, akong, qemu-devel, mdroth
[-- Attachment #1: Type: text/plain, Size: 1225 bytes --]
On 07/26/2013 01:35 PM, Markus Armbruster wrote:
>>> + if not self.tok in [ '{', '[', "'" ]:
>>> + raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
>>
>> JSON allows primitives here, as in [ 1 ]; but I agree that for the
>> purposes of our schema we will always be taking a string or complex
>> object whenever we have a list.
>
> The lexer doesn't recognize any atoms but strings. If we change that,
> the syntax error messages need to be reviewed (not just this one).
Nah, I'm fine leaving the lexer as-is. It's okay that we parse only a
subset of JSON, as long as our subset is expressive enough for our needs
in QAPI.
As it is, technically, we aren't QUITE parsing JSON, because our schema
strings are marked with '' instead of "". And as long as we are
extending our parser to take something slightly different than JSON,
maybe we should teach it to tolerate trailing commas? On the other
hand, the further we diverge from JSON, the more likely we are to have
to maintain the parser ourselves instead of being able to reuse someone
else's code.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 621 bytes --]
^ permalink raw reply [flat|nested] 32+ messages in thread
* [Qemu-devel] [PATCH 7/9] qapi.py: Fix diagnosing non-objects at a schema's top-level
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (5 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 6/9] qapi.py: Fix schema parser to check syntax systematically Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 16:03 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 8/9] qapi.py: Rename expr_eval to expr in parse_schema() Markus Armbruster
` (2 subsequent siblings)
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Report syntax error instead of crashing.
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 10 ++++++----
tests/qapi-schema/non-objects.err | 2 +-
tests/qapi-schema/quoted-structural-chars.err | 2 +-
3 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index a7feccb..5677daa 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -49,7 +49,7 @@ class QAPISchema:
self.accept()
while self.tok != None:
- self.exprs.append(self.get_expr())
+ self.exprs.append(self.get_expr(False))
def accept(self):
while True:
@@ -102,7 +102,7 @@ class QAPISchema:
if self.tok != ':':
raise QAPISchemaError(self, 'Expected ":"')
self.accept()
- expr[key] = self.get_expr()
+ expr[key] = self.get_expr(True)
if self.tok == '}':
self.accept()
return expr
@@ -120,7 +120,7 @@ class QAPISchema:
if not self.tok in [ '{', '[', "'" ]:
raise QAPISchemaError(self, 'Expected "{", "[", "]" or string')
while True:
- expr.append(self.get_expr())
+ expr.append(self.get_expr(True))
if self.tok == ']':
self.accept()
return expr
@@ -128,7 +128,9 @@ class QAPISchema:
raise QAPISchemaError(self, 'Expected "," or "]"')
self.accept()
- def get_expr(self):
+ def get_expr(self, nested):
+ if self.tok != '{' and not nested:
+ raise QAPISchemaError(self, 'Expected "{"')
if self.tok == '{':
self.accept()
expr = self.get_members()
diff --git a/tests/qapi-schema/non-objects.err b/tests/qapi-schema/non-objects.err
index 48c849d..a6c2dc2 100644
--- a/tests/qapi-schema/non-objects.err
+++ b/tests/qapi-schema/non-objects.err
@@ -1 +1 @@
-Crashed: <type 'exceptions.AttributeError'>
+<stdin>:1:1: Expected "{"
diff --git a/tests/qapi-schema/quoted-structural-chars.err b/tests/qapi-schema/quoted-structural-chars.err
index 48c849d..a6c2dc2 100644
--- a/tests/qapi-schema/quoted-structural-chars.err
+++ b/tests/qapi-schema/quoted-structural-chars.err
@@ -1 +1 @@
-Crashed: <type 'exceptions.AttributeError'>
+<stdin>:1:1: Expected "{"
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 8/9] qapi.py: Rename expr_eval to expr in parse_schema()
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (6 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 7/9] qapi.py: Fix diagnosing non-objects at a schema's top-level Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 16:13 ` Eric Blake
2013-07-26 12:39 ` [Qemu-devel] [PATCH 9/9] qapi.py: Permit comments starting anywhere on the line Markus Armbruster
2013-07-26 14:41 ` [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Anthony Liguori
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 16 ++++++++--------
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 5677daa..1d856c9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -153,14 +153,14 @@ def parse_schema(fp):
exprs = []
- for expr_eval in schema.exprs:
- if expr_eval.has_key('enum'):
- add_enum(expr_eval['enum'])
- elif expr_eval.has_key('union'):
- add_enum('%sKind' % expr_eval['union'])
- elif expr_eval.has_key('type'):
- add_struct(expr_eval)
- exprs.append(expr_eval)
+ for expr in schema.exprs:
+ if expr.has_key('enum'):
+ add_enum(expr['enum'])
+ elif expr.has_key('union'):
+ add_enum('%sKind' % expr['union'])
+ elif expr.has_key('type'):
+ add_struct(expr)
+ exprs.append(expr)
return exprs
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* [Qemu-devel] [PATCH 9/9] qapi.py: Permit comments starting anywhere on the line
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (7 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 8/9] qapi.py: Rename expr_eval to expr in parse_schema() Markus Armbruster
@ 2013-07-26 12:39 ` Markus Armbruster
2013-07-26 16:15 ` Eric Blake
2013-07-26 14:41 ` [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Anthony Liguori
9 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 12:39 UTC (permalink / raw)
To: qemu-devel; +Cc: aliguori, akong, mdroth
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
scripts/qapi.py | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/scripts/qapi.py b/scripts/qapi.py
index 1d856c9..da46fb9 100644
--- a/scripts/qapi.py
+++ b/scripts/qapi.py
@@ -53,13 +53,12 @@ class QAPISchema:
def accept(self):
while True:
- bol = self.cursor == 0 or self.src[self.cursor-1] == '\n'
self.tok = self.src[self.cursor]
self.pos = self.cursor
self.cursor += 1
self.val = None
- if self.tok == '#' and bol:
+ if self.tok == '#':
self.cursor = self.src.find('\n', self.cursor)
elif self.tok in ['{', '}', ':', ',', '[', ']']:
return
--
1.7.11.7
^ permalink raw reply related [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it
2013-07-26 12:39 [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Markus Armbruster
` (8 preceding siblings ...)
2013-07-26 12:39 ` [Qemu-devel] [PATCH 9/9] qapi.py: Permit comments starting anywhere on the line Markus Armbruster
@ 2013-07-26 14:41 ` Anthony Liguori
2013-07-26 15:36 ` Markus Armbruster
9 siblings, 1 reply; 32+ messages in thread
From: Anthony Liguori @ 2013-07-26 14:41 UTC (permalink / raw)
To: Markus Armbruster, qemu-devel; +Cc: akong, mdroth
Markus Armbruster <armbru@redhat.com> writes:
> If you think I'm exaggerating, check out the list of issues in PATCH
> 3/9.
You are not.
However, I think we can drop the whole thing and just use the JSON
module in Python. The bit below seems to work:
import json.decoder, re
from ordereddict import OrderedDict
WHITESPACE = re.compile(r'(#.*\n|[ \r\t\n]*)*', re.MULTILINE)
def make_object(pairs):
return OrderedDict(pairs)
def qapi_parse(data):
_w = WHITESPACE.match
idx = 0
while idx < len(data):
idx = _w(data, idx).end()
if idx == len(data):
break
decoder = json.decoder.JSONDecoder(object_pairs_hook=make_object)
obj, idx = decoder.raw_decode(data, idx)
yield obj
if __name__ == '__main__':
with open('qapi-schema.json', 'r') as fp:
data = fp.read().replace("'", '"')
exprs = list(qapi_parse(data))
print exprs
Regards,
Anthony Liguori
>
> Markus Armbruster (9):
> tests: QAPI schema parser tests
> tests: Use qapi-schema-test.json as schema parser test
> qapi.py: Restructure lexer and parser
> qapi.py: Decent syntax error reporting
> qapi.py: Reject invalid characters in schema file
> qapi.py: Fix schema parser to check syntax systematically
> qapi.py: Fix diagnosing non-objects at a schema's top-level
> qapi.py: Rename expr_eval to expr in parse_schema()
> qapi.py: Permit comments starting anywhere on the line
>
> configure | 2 +-
> qapi-schema-test.json | 53 ------
> scripts/qapi.py | 225 +++++++++++++++----------
> tests/Makefile | 28 ++-
> tests/qapi-schema/empty.exit | 1 +
> tests/qapi-schema/empty.out | 3 +
> tests/qapi-schema/funny-char.err | 1 +
> tests/qapi-schema/funny-char.exit | 1 +
> tests/qapi-schema/funny-char.json | 2 +
> tests/qapi-schema/indented-expr.exit | 1 +
> tests/qapi-schema/indented-expr.json | 2 +
> tests/qapi-schema/indented-expr.out | 3 +
> tests/qapi-schema/missing-colon.err | 1 +
> tests/qapi-schema/missing-colon.exit | 1 +
> tests/qapi-schema/missing-colon.json | 2 +
> tests/qapi-schema/missing-comma.err | 1 +
> tests/qapi-schema/missing-comma.exit | 1 +
> tests/qapi-schema/missing-comma.json | 2 +
> tests/qapi-schema/non-objects.err | 1 +
> tests/qapi-schema/non-objects.exit | 1 +
> tests/qapi-schema/non-objects.json | 2 +
> tests/qapi-schema/qapi-schema-test.exit | 1 +
> tests/qapi-schema/qapi-schema-test.json | 53 ++++++
> tests/qapi-schema/qapi-schema-test.out | 19 +++
> tests/qapi-schema/quoted-structural-chars.err | 1 +
> tests/qapi-schema/quoted-structural-chars.exit | 1 +
> tests/qapi-schema/quoted-structural-chars.json | 1 +
> tests/qapi-schema/test-qapi.py | 27 +++
> tests/qapi-schema/unclosed-object.err | 1 +
> tests/qapi-schema/unclosed-object.exit | 1 +
> tests/qapi-schema/unclosed-object.json | 1 +
> tests/qapi-schema/unclosed-string.err | 1 +
> tests/qapi-schema/unclosed-string.exit | 1 +
> tests/qapi-schema/unclosed-string.json | 2 +
> 34 files changed, 298 insertions(+), 146 deletions(-)
> delete mode 100644 qapi-schema-test.json
> create mode 100644 tests/qapi-schema/empty.err
> create mode 100644 tests/qapi-schema/empty.exit
> create mode 100644 tests/qapi-schema/empty.json
> create mode 100644 tests/qapi-schema/empty.out
> create mode 100644 tests/qapi-schema/funny-char.err
> create mode 100644 tests/qapi-schema/funny-char.exit
> create mode 100644 tests/qapi-schema/funny-char.json
> create mode 100644 tests/qapi-schema/funny-char.out
> create mode 100644 tests/qapi-schema/indented-expr.err
> create mode 100644 tests/qapi-schema/indented-expr.exit
> create mode 100644 tests/qapi-schema/indented-expr.json
> create mode 100644 tests/qapi-schema/indented-expr.out
> create mode 100644 tests/qapi-schema/missing-colon.err
> create mode 100644 tests/qapi-schema/missing-colon.exit
> create mode 100644 tests/qapi-schema/missing-colon.json
> create mode 100644 tests/qapi-schema/missing-colon.out
> create mode 100644 tests/qapi-schema/missing-comma.err
> create mode 100644 tests/qapi-schema/missing-comma.exit
> create mode 100644 tests/qapi-schema/missing-comma.json
> create mode 100644 tests/qapi-schema/missing-comma.out
> create mode 100644 tests/qapi-schema/non-objects.err
> create mode 100644 tests/qapi-schema/non-objects.exit
> create mode 100644 tests/qapi-schema/non-objects.json
> create mode 100644 tests/qapi-schema/non-objects.out
> create mode 100644 tests/qapi-schema/qapi-schema-test.err
> create mode 100644 tests/qapi-schema/qapi-schema-test.exit
> create mode 100644 tests/qapi-schema/qapi-schema-test.json
> create mode 100644 tests/qapi-schema/qapi-schema-test.out
> create mode 100644 tests/qapi-schema/quoted-structural-chars.err
> create mode 100644 tests/qapi-schema/quoted-structural-chars.exit
> create mode 100644 tests/qapi-schema/quoted-structural-chars.json
> create mode 100644 tests/qapi-schema/quoted-structural-chars.out
> create mode 100644 tests/qapi-schema/test-qapi.py
> create mode 100644 tests/qapi-schema/unclosed-object.err
> create mode 100644 tests/qapi-schema/unclosed-object.exit
> create mode 100644 tests/qapi-schema/unclosed-object.json
> create mode 100644 tests/qapi-schema/unclosed-object.out
> create mode 100644 tests/qapi-schema/unclosed-string.err
> create mode 100644 tests/qapi-schema/unclosed-string.exit
> create mode 100644 tests/qapi-schema/unclosed-string.json
> create mode 100644 tests/qapi-schema/unclosed-string.out
>
> --
> 1.7.11.7
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it
2013-07-26 14:41 ` [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it Anthony Liguori
@ 2013-07-26 15:36 ` Markus Armbruster
2013-07-26 17:47 ` Anthony Liguori
0 siblings, 1 reply; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 15:36 UTC (permalink / raw)
To: Anthony Liguori; +Cc: akong, qemu-devel, mdroth
Anthony Liguori <aliguori@us.ibm.com> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> If you think I'm exaggerating, check out the list of issues in PATCH
>> 3/9.
>
> You are not.
>
> However, I think we can drop the whole thing and just use the JSON
> module in Python. The bit below seems to work:
>
> import json.decoder, re
> from ordereddict import OrderedDict
>
> WHITESPACE = re.compile(r'(#.*\n|[ \r\t\n]*)*', re.MULTILINE)
>
> def make_object(pairs):
> return OrderedDict(pairs)
>
> def qapi_parse(data):
> _w = WHITESPACE.match
> idx = 0
> while idx < len(data):
> idx = _w(data, idx).end()
> if idx == len(data):
> break
> decoder = json.decoder.JSONDecoder(object_pairs_hook=make_object)
> obj, idx = decoder.raw_decode(data, idx)
> yield obj
>
> if __name__ == '__main__':
> with open('qapi-schema.json', 'r') as fp:
> data = fp.read().replace("'", '"')
>
> exprs = list(qapi_parse(data))
> print exprs
I tried to find a way to use JSONDecoder, but not hard enough,
apparently.
The fp.read().replace("'", '"') is no good, because it blindly replaces
within strings, such as 'the cat\'s meow'.
Can your code handle comments between arbitrary tokens? I suspect they
work only between top-level expressions, but I could be wrong; Python
isn't my strongest language, and I didn't test this.
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it
2013-07-26 15:36 ` Markus Armbruster
@ 2013-07-26 17:47 ` Anthony Liguori
2013-07-26 19:48 ` Markus Armbruster
0 siblings, 1 reply; 32+ messages in thread
From: Anthony Liguori @ 2013-07-26 17:47 UTC (permalink / raw)
To: Markus Armbruster; +Cc: akong, qemu-devel, mdroth
Markus Armbruster <armbru@redhat.com> writes:
> Anthony Liguori <aliguori@us.ibm.com> writes:
>
>> Markus Armbruster <armbru@redhat.com> writes:
>>
>>> If you think I'm exaggerating, check out the list of issues in PATCH
>>> 3/9.
>>
>> You are not.
>>
>> However, I think we can drop the whole thing and just use the JSON
>> module in Python. The bit below seems to work:
>>
>> import json.decoder, re
>> from ordereddict import OrderedDict
>>
>> WHITESPACE = re.compile(r'(#.*\n|[ \r\t\n]*)*', re.MULTILINE)
>>
>> def make_object(pairs):
>> return OrderedDict(pairs)
>>
>> def qapi_parse(data):
>> _w = WHITESPACE.match
>> idx = 0
>> while idx < len(data):
>> idx = _w(data, idx).end()
>> if idx == len(data):
>> break
>> decoder = json.decoder.JSONDecoder(object_pairs_hook=make_object)
>> obj, idx = decoder.raw_decode(data, idx)
>> yield obj
>>
>> if __name__ == '__main__':
>> with open('qapi-schema.json', 'r') as fp:
>> data = fp.read().replace("'", '"')
>>
>> exprs = list(qapi_parse(data))
>> print exprs
>
> I tried to find a way to use JSONDecoder, but not hard enough,
> apparently.
>
> The fp.read().replace("'", '"') is no good, because it blindly replaces
> within strings, such as 'the cat\'s meow'.
>
> Can your code handle comments between arbitrary tokens? I suspect they
> work only between top-level expressions, but I could be wrong; Python
> isn't my strongest language, and I didn't test this.
It cannot. The python JSON module has an optimized C implementation
which is less flexible than the python one. Unfortunately it looks like
at least in my copy of python, the python version has bitrotted.
OTOH, the warnings are very clear when you attempt to do this.
Regards,
Anthony Liguori
^ permalink raw reply [flat|nested] 32+ messages in thread* Re: [Qemu-devel] [PATCH 0/9] Our QAPI parser is a hack, replace it
2013-07-26 17:47 ` Anthony Liguori
@ 2013-07-26 19:48 ` Markus Armbruster
0 siblings, 0 replies; 32+ messages in thread
From: Markus Armbruster @ 2013-07-26 19:48 UTC (permalink / raw)
To: Anthony Liguori; +Cc: akong, qemu-devel, mdroth
Anthony Liguori <aliguori@us.ibm.com> writes:
> Markus Armbruster <armbru@redhat.com> writes:
>
>> Anthony Liguori <aliguori@us.ibm.com> writes:
>>
>>> Markus Armbruster <armbru@redhat.com> writes:
>>>
>>>> If you think I'm exaggerating, check out the list of issues in PATCH
>>>> 3/9.
>>>
>>> You are not.
>>>
>>> However, I think we can drop the whole thing and just use the JSON
>>> module in Python. The bit below seems to work:
>>>
>>> import json.decoder, re
>>> from ordereddict import OrderedDict
>>>
>>> WHITESPACE = re.compile(r'(#.*\n|[ \r\t\n]*)*', re.MULTILINE)
>>>
>>> def make_object(pairs):
>>> return OrderedDict(pairs)
>>>
>>> def qapi_parse(data):
>>> _w = WHITESPACE.match
>>> idx = 0
>>> while idx < len(data):
>>> idx = _w(data, idx).end()
>>> if idx == len(data):
>>> break
>>> decoder = json.decoder.JSONDecoder(object_pairs_hook=make_object)
>>> obj, idx = decoder.raw_decode(data, idx)
>>> yield obj
>>>
>>> if __name__ == '__main__':
>>> with open('qapi-schema.json', 'r') as fp:
>>> data = fp.read().replace("'", '"')
>>>
>>> exprs = list(qapi_parse(data))
>>> print exprs
>>
>> I tried to find a way to use JSONDecoder, but not hard enough,
>> apparently.
>>
>> The fp.read().replace("'", '"') is no good, because it blindly replaces
>> within strings, such as 'the cat\'s meow'.
>>
>> Can your code handle comments between arbitrary tokens? I suspect they
>> work only between top-level expressions, but I could be wrong; Python
>> isn't my strongest language, and I didn't test this.
>
> It cannot. The python JSON module has an optimized C implementation
> which is less flexible than the python one. Unfortunately it looks like
> at least in my copy of python, the python version has bitrotted.
>
> OTOH, the warnings are very clear when you attempt to do this.
Outlawing comments within expressions feels very wrong to me.
An example of such a comment is in my [PATCH v2 3/3] qapi: Rename
ChardevBackend member "memory" to "ringbuf".
Since JSON's lexical structure is so simple, you could strip comments
with a simple state machine, or maybe with a (not so simple) regexp.
Basically a stripped down JSON lexer. Then using JSONDecoder still
saves maintaining a parser, but not a lexer.
You'd trade maintaining a pretty trivial parser for maintaining not so
trivial JSONDecoder glue.
^ permalink raw reply [flat|nested] 32+ messages in thread