VladPolskiy/qemu
1
0
Fork 0
mirror of https://github.com/qemu/qemu.git synced 2026-01-14 03:01:08 +00:00
Code Issues Packages Projects Releases Wiki Activity

qapi/command: Avoid generating unused qmp_marshal_output_T()

Browse Source 
qmp_marshal_output_T() is only ever called by qmp_marshal_C() for a
command C that returns type T.

We've always generated it as a static function on demand, i.e. when we
generate a call.

Since we split up monolithic generated code into modules (commit
252dc3105f "qapi: Generate separate .h, .c for each module"), we do
this per module.  As noted in the commit message, this can result in
identical (static) qmp_marshal_output_T() in several modules.  Was
deemed not worth avoiding.

A bit later, we added 'if' conditionals to the schema language (merge
commit 5dafaf4fbc).

When a conditional definition uses a type, then its condition must
imply the type's condition.  We made this the user's responsibility.
Hasn't been an issue in practice.

However, the sharing of qmp_marshal_output_T() among commands
complicates matters.  To avoid both undefined function errors and
unused function warnings, qmp_marshal_output_T() must be defined
exactly when it's used.  It is used when any of the qmp_marshal_C()
calling it is defined, i.e. when any C's condition holds.

The generator uses T's condition instead.  To avoid both error and
warning, T's condition must be the conjunction of all C's conditions.

Unfortunately, this can be impossible:

* Conditional command returning a builtin type

  A builtin type cannot be conditional.  This is noted in a FIXME
  comment.

* Commands in multiple modules where the conjunction differs between
  modules

  An instance of this came up recently.  we have unconditional
  commands returning HumanReadableText.  If we add a conditional one
  to a module that does not have unconditional ones, compilation fails
  with "defined but not used".  If we make HumanReadableText
  conditional to fix this module, we break the others.

Instead of complicating the code to compute the conjunction, simplify
it: generate the output marshaling code right into qmp_marshal_C().

This duplicates it when multiple commands return the same type.  The
impact on code size is negligible: qemu-system-x86_64's text segment
grows by 1448 bytes.

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-ID: <20250804130602.903904-1-armbru@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
[Commit message typos fixed]
This commit is contained in:
Markus Armbruster
2025-08-04 15:06:02 +02:00
parent a8e63c0130
commit 5bd89761a4
2 changed files with 19 additions and 50 deletions
Download Patch File Download Diff File Expand all files Collapse all files

25
docs/devel/qapi-code-gen.rst
View File

@ -1809,27 +1809,13 @@ Example::
$ cat qapi-generated/example-qapi-commands.c
[Uninteresting stuff omitted...]
static void qmp_marshal_output_UserDefOne(UserDefOne *ret_in,
QObject **ret_out, Error **errp)
{
Visitor *v;
v = qobject_output_visitor_new_qmp(ret_out);
if (visit_type_UserDefOne(v, "unused", &ret_in, errp)) {
visit_complete(v, ret_out);
}
visit_free(v);
v = qapi_dealloc_visitor_new();
visit_type_UserDefOne(v, "unused", &ret_in, NULL);
visit_free(v);
}
void qmp_marshal_my_command(QDict *args, QObject **ret, Error **errp)
{
Error *err = NULL;
bool ok = false;
Visitor *v;
UserDefOne *retval;
Visitor *ov;
q_obj_my_command_arg arg = {0};
v = qobject_input_visitor_new_qmp(QOBJECT(args));
@ -1857,7 +1843,14 @@ Example::
goto out;
}
qmp_marshal_output_UserDefOne(retval, ret, errp);
ov = qobject_output_visitor_new_qmp(ret);
if (visit_type_UserDefOne(ov, "unused", &retval, errp)) {
visit_complete(ov, ret);
}
visit_free(ov);
ov = qapi_dealloc_visitor_new();
visit_type_UserDefOne(ov, "unused", &retval, NULL);
visit_free(ov);
if (trace_event_get_state_backends(TRACE_QMP_EXIT_MY_COMMAND)) {
g_autoptr(GString) ret_json = qobject_to_json(*ret);

44
scripts/qapi/commands.py
View File

@ -14,15 +14,12 @@ See the COPYING file in the top-level directory.
"""
from typing import (
Dict,
List,
Optional,
Set,
)
from .common import c_name, mcgen
from .gen import (
QAPIGenC,
QAPISchemaModularCVisitor,
build_params,
gen_features,
@ -112,11 +109,7 @@ def gen_call(name: str,
''')
if ret_type:
ret += mcgen('''
qmp_marshal_output_%(c_name)s(retval, ret, errp);
''',
c_name=ret_type.c_name())
ret += gen_marshal_output(ret_type)
if gen_tracing:
if ret_type:
@ -142,22 +135,16 @@ def gen_call(name: str,
def gen_marshal_output(ret_type: QAPISchemaType) -> str:
return mcgen('''
static void qmp_marshal_output_%(c_name)s(%(c_type)s ret_in,
QObject **ret_out, Error **errp)
{
Visitor *v;
v = qobject_output_visitor_new_qmp(ret_out);
if (visit_type_%(c_name)s(v, "unused", &ret_in, errp)) {
visit_complete(v, ret_out);
ov = qobject_output_visitor_new_qmp(ret);
if (visit_type_%(c_name)s(ov, "unused", &retval, errp)) {
visit_complete(ov, ret);
}
visit_free(v);
v = qapi_dealloc_visitor_new();
visit_type_%(c_name)s(v, "unused", &ret_in, NULL);
visit_free(v);
}
visit_free(ov);
ov = qapi_dealloc_visitor_new();
visit_type_%(c_name)s(ov, "unused", &retval, NULL);
visit_free(ov);
''',
c_type=ret_type.c_type(), c_name=ret_type.c_name())
c_name=ret_type.c_name())
def build_marshal_proto(name: str,
@ -209,6 +196,7 @@ def gen_marshal(name: str,
if ret_type:
ret += mcgen('''
%(c_type)s retval;
Visitor *ov;
''',
c_type=ret_type.c_type())
@ -308,11 +296,9 @@ class QAPISchemaGenCommandVisitor(QAPISchemaModularCVisitor):
prefix, 'qapi-commands',
' * Schema-defined QAPI/QMP commands', None, __doc__,
gen_tracing=gen_tracing)
self._visited_ret_types: Dict[QAPIGenC, Set[QAPISchemaType]] = {}
self._gen_tracing = gen_tracing
def _begin_user_module(self, name: str) -> None:
self._visited_ret_types[self._genc] = set()
commands = self._module_basename('qapi-commands', name)
types = self._module_basename('qapi-types', name)
visit = self._module_basename('qapi-visit', name)
@ -386,16 +372,6 @@ void %(c_prefix)sqmp_init_marshal(QmpCommandList *cmds)
coroutine: bool) -> None:
if not gen:
return
# FIXME: If T is a user-defined type, the user is responsible
# for making this work, i.e. to make T's condition the
# conjunction of the T-returning commands' conditions. If T
# is a built-in type, this isn't possible: the
# qmp_marshal_output_T() will be generated unconditionally.
if ret_type and ret_type not in self._visited_ret_types[self._genc]:
self._visited_ret_types[self._genc].add(ret_type)
with ifcontext(ret_type.ifcond,
self._genh, self._genc):
self._genc.add(gen_marshal_output(ret_type))
with ifcontext(ifcond, self._genh, self._genc):
self._genh.add(gen_command_decl(name, arg_type, boxed,
ret_type, coroutine))