[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Add comments to aid debugging gene
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH v2 2/2] qapi: Add comments to aid debugging generated introspection |
Date: |
Tue, 28 Aug 2018 14:27:10 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.1 (gnu/linux) |
Eric Blake <address@hidden> writes:
> We consciously chose in commit 1a9a507b to hide QAPI type names
> from the introspection output on the wire, but added a command
> line option -u to unmask the type name when doing a debug build.
> The unmask option still remains useful to some other forms of
> automated analysis, so it will not be removed; however, when it
> is not in use, the generated .c file can be hard to read. At
> the time when we first introduced masking, the generated file
> consisted only of a monolithic C string, so there was no clean
> way to inject any comments.
>
> Later, in commit 7d0f982b, we switched the generation to output
> a QLit object, in part to make it easier for future addition of
> conditional compilation. In fact, commit d626b6c1 took advantage
> of this by passing a tuple instead of a bare object for encoding
> the output of conditionals. By extending that tuple, we can now
> interject strategic comments.
>
> For now, type name debug aid comments are only output once per
> meta-type, rather than at all uses of the number used to encode
> the type within the introspection data. But this is still a lot
> more convenient than having to regenerate the file with the
> unmask operation temporarily turned on - merely search the
> generated file for '"NNN" =' to learn the corresponding source
> name and associated definition of type NNN.
>
> The generated qapi-introspect.c changes only with the addition
> of comments, such as:
>
> | @@ -14755,6 +15240,7 @@
> | { "name", QLIT_QSTR("[485]"), },
> | {}
> | })),
> | + /* "485" = QCryptoBlockInfoLUKSSlot */
> | QLIT_QDICT(((QLitDictEntry[]) {
> | { "members", QLIT_QLIST(((QLitObject[]) {
> | QLIT_QDICT(((QLitDictEntry[]) {
>
> Signed-off-by: Eric Blake <address@hidden>
>
> ---
> v2: rebase on conditional code additions, drop patch to remove -u,
> update documentation to match
> ---
> docs/devel/qapi-code-gen.txt | 1 +
> scripts/qapi/introspect.py | 27 +++++++++++++++++++++------
> 2 files changed, 22 insertions(+), 6 deletions(-)
>
> diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
> index c2e11465f0f..9d5b1409e72 100644
> --- a/docs/devel/qapi-code-gen.txt
> +++ b/docs/devel/qapi-code-gen.txt
> @@ -1412,6 +1412,7 @@ Example:
> { "name", QLIT_QSTR("Event") },
> { }
> })),
> + /* "0" = q_empty */
> QLIT_QDICT(((QLitDictEntry[]) {
> { "members", QLIT_QLIST(((QLitObject[]) {
> { }
Let's rebase onto my "[PATCH 0/2] qapi: A whitespace touch-up, and a doc
update". The appended fixup needs to be squashed in then. Happy to do
that in my tree.
> diff --git a/scripts/qapi/introspect.py b/scripts/qapi/introspect.py
> index 43e81a06938..67d6106f77b 100644
> --- a/scripts/qapi/introspect.py
> +++ b/scripts/qapi/introspect.py
> @@ -19,12 +19,17 @@ def to_qlit(obj, level=0, suppress_first_indent=False):
> return level * 4 * ' '
>
> if isinstance(obj, tuple):
> - ifobj, ifcond = obj
> - ret = gen_if(ifcond)
> + ifobj, extra = obj
> + ifcond = extra.get('if')
> + comment = extra.get('comment')
> + ret = ''
> + if comment:
> + ret += indent(level) + '/* %s */\n' % comment
> + if ifcond:
> + ret += gen_if(ifcond)
> ret += to_qlit(ifobj, level)
> - endif = gen_endif(ifcond)
> - if endif:
> - ret += '\n' + endif
> + if ifcond:
> + ret += '\n' + gen_endif(ifcond)
> return ret
>
> ret = ''
> @@ -137,11 +142,21 @@ const QLitObject %(c_name)s = %(c_string)s;
> return self._name(typ.name)
>
> def _gen_qlit(self, name, mtype, obj, ifcond):
> + extra = {}
> if mtype not in ('command', 'event', 'builtin', 'array'):
> + if not self._unmask:
> + # Output a comment to make it easy to map masked names
> + # back to the source when reading the generated output.
> + extra['comment'] = '"%s" = %s' % (self._name(name), name)
> name = self._name(name)
> obj['name'] = name
> obj['meta-type'] = mtype
> - self._qlits.append((obj, ifcond))
> + if ifcond:
> + extra['if'] = ifcond
> + if extra:
> + self._qlits.append((obj, extra))
> + else:
> + self._qlits.append(obj)
>
> def _gen_member(self, member):
> ret = {'name': member.name, 'type': self._use_type(member.type)}
Reviewed-by: Markus Armbruster <address@hidden>
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index a4091a7d78..f9990808de 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1430,7 +1430,7 @@ Example:
{ "name", QLIT_QSTR("MY_EVENT"), },
{}
})),
- /* "0" = q_empty */
+ /* "0" = q_obj_my-command-arg */
QLIT_QDICT(((QLitDictEntry[]) {
{ "members", QLIT_QLIST(((QLitObject[]) {
QLIT_QDICT(((QLitDictEntry[]) {
@@ -1444,6 +1444,7 @@ Example:
{ "name", QLIT_QSTR("0"), },
{}
})),
+ /* "1" = UserDefOne */
QLIT_QDICT(((QLitDictEntry[]) {
{ "members", QLIT_QLIST(((QLitObject[]) {
QLIT_QDICT(((QLitDictEntry[]) {
@@ -1463,6 +1464,7 @@ Example:
{ "name", QLIT_QSTR("1"), },
{}
})),
+ /* "2" = q_empty */
QLIT_QDICT(((QLitDictEntry[]) {
{ "members", QLIT_QLIST(((QLitObject[]) {
{}