aboutsummaryrefslogtreecommitdiff
path: root/include/qapi/qmp
diff options
context:
space:
mode:
authorMarkus Armbruster <armbru@redhat.com>2023-09-21 14:13:12 +0200
committerMarkus Armbruster <armbru@redhat.com>2023-09-29 08:13:57 +0200
commitbb71846325e23d884ca4ff1bcc95aaead0131a5a (patch)
treeedac10c4aece602edc2fcd646ba97e0a73c0be73 /include/qapi/qmp
parentfb2575f95411644abe7f0606594035b63a5132ad (diff)
qobject atomics osdep: Make a few macros more hygienic
Variables declared in macros can shadow other variables. Much of the time, this is harmless, e.g.: #define _FDT(exp) \ do { \ int ret = (exp); \ if (ret < 0) { \ error_report("error creating device tree: %s: %s", \ #exp, fdt_strerror(ret)); \ exit(1); \ } \ } while (0) Harmless shadowing in h_client_architecture_support(): target_ulong ret; [...] ret = do_client_architecture_support(cpu, spapr, vec, fdt_bufsize); if (ret == H_SUCCESS) { _FDT((fdt_pack(spapr->fdt_blob))); [...] } return ret; However, we can get in trouble when the shadowed variable is used in a macro argument: #define QOBJECT(obj) ({ \ typeof(obj) o = (obj); \ o ? container_of(&(o)->base, QObject, base) : NULL; \ }) QOBJECT(o) expands into ({ ---> typeof(o) o = (o); o ? container_of(&(o)->base, QObject, base) : NULL; }) Unintended variable name capture at --->. We'd be saved by -Winit-self. But I could certainly construct more elaborate death traps that don't trigger it. To reduce the risk of trapping ourselves, we use variable names in macros that no sane person would use elsewhere. Here's our actual definition of QOBJECT(): #define QOBJECT(obj) ({ \ typeof(obj) _obj = (obj); \ _obj ? container_of(&(_obj)->base, QObject, base) : NULL; \ }) Works well enough until we nest macro calls. For instance, with #define qobject_ref(obj) ({ \ typeof(obj) _obj = (obj); \ qobject_ref_impl(QOBJECT(_obj)); \ _obj; \ }) the expression qobject_ref(obj) expands into ({ typeof(obj) _obj = (obj); qobject_ref_impl( ({ ---> typeof(_obj) _obj = (_obj); _obj ? container_of(&(_obj)->base, QObject, base) : NULL; })); _obj; }) Unintended variable name capture at --->. The only reliable way to prevent unintended variable name capture is -Wshadow. One blocker for enabling it is shadowing hiding in function-like macros like qdict_put(dict, "name", qobject_ref(...)) qdict_put() wraps its last argument in QOBJECT(), and the last argument here contains another QOBJECT(). Use dark preprocessor sorcery to make the macros that give us this problem use different variable names on every call. Signed-off-by: Markus Armbruster <armbru@redhat.com> Reviewed-by: Eric Blake <eblake@redhat.com> Message-ID: <20230921121312.1301864-8-armbru@redhat.com> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Diffstat (limited to 'include/qapi/qmp')
-rw-r--r--include/qapi/qmp/qobject.h10
1 files changed, 8 insertions, 2 deletions
diff --git a/include/qapi/qmp/qobject.h b/include/qapi/qmp/qobject.h
index 9003b71fd3..89b97d88bc 100644
--- a/include/qapi/qmp/qobject.h
+++ b/include/qapi/qmp/qobject.h
@@ -45,10 +45,16 @@ struct QObject {
struct QObjectBase_ base;
};
-#define QOBJECT(obj) ({ \
+/*
+ * Preprocessor sorcery ahead: use a different identifier for the
+ * local variable in each expansion, so we can nest macro calls
+ * without shadowing variables.
+ */
+#define QOBJECT_INTERNAL(obj, _obj) ({ \
typeof(obj) _obj = (obj); \
- _obj ? container_of(&(_obj)->base, QObject, base) : NULL; \
+ _obj ? container_of(&_obj->base, QObject, base) : NULL; \
})
+#define QOBJECT(obj) QOBJECT_INTERNAL((obj), MAKE_IDENTFIER(_obj))
/* Required for qobject_to() */
#define QTYPE_CAST_TO_QNull QTYPE_QNULL