[PATCH] error: Rewrite &error_warn with ERRP_GUARD()

Akihiko Odaki posted 1 patch 3 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20250803-errp-v1-1-a59a73f63289@rsg.ci.i.u-tokyo.ac.jp
Maintainers: Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>
include/qapi/error.h | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
[PATCH] error: Rewrite &error_warn with ERRP_GUARD()
Posted by Akihiko Odaki 3 months, 1 week ago
Without rewrite, an warning message may be printed before
error_prepend(); in such a scenario, error_prepend() will results in
NULL dereference because error_warn is NULL.

Rewrite &error_warn with ERRP_GUARD() to make sure error_prepend() is
reflected to printed messages and to avoid NULL dereference.

Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
---
 include/qapi/error.h | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index 41e38163804904874e5d114b699a3e2a93d3fc6d..4fa04980c2fd110aa68ced020dd1ec8a83d7d957 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -505,20 +505,20 @@ void error_set_internal(Error **errp,
  * It is safe to use even when it's not needed, but please avoid
  * cluttering the source with useless code.
  *
- * If @errp is NULL or &error_fatal, rewrite it to point to a local
- * Error variable, which will be automatically propagated to the
- * original @errp on function exit.
+ * If @errp is NULL, &error_fatal, or &error_warn, rewrite it to point
+ * to a local Error variable, which will be automatically propagated to
+ * the original @errp on function exit.
  *
  * Note: &error_abort is not rewritten, because that would move the
  * abort from the place where the error is created to the place where
  * it's propagated.
  */
-#define ERRP_GUARD()                                            \
-    g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp};   \
-    do {                                                        \
-        if (!errp || errp == &error_fatal) {                    \
-            errp = &_auto_errp_prop.local_err;                  \
-        }                                                       \
+#define ERRP_GUARD()                                                \
+    g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp};       \
+    do {                                                            \
+        if (!errp || errp == &error_fatal || errp == &error_warn) { \
+            errp = &_auto_errp_prop.local_err;                      \
+        }                                                           \
     } while (0)
 
 typedef struct ErrorPropagator {

---
base-commit: f0737158b483e7ec2b2512145aeab888b85cc1f7
change-id: 20250803-errp-e54c8242e2dd

Best regards,
-- 
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Re: [PATCH] error: Rewrite &error_warn with ERRP_GUARD()
Posted by Markus Armbruster 3 months, 1 week ago
Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> writes:

> Without rewrite, an warning message may be printed before
> error_prepend(); in such a scenario, error_prepend() will results in
> NULL dereference because error_warn is NULL.
>
> Rewrite &error_warn with ERRP_GUARD() to make sure error_prepend() is
> reflected to printed messages and to avoid NULL dereference.
>
> Signed-off-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>

You're right.  Needs

  Fixes: 3ffef1a55ca3 (error: add global &error_warn destination)

However, there's still more to fix up, and &error_warn is rarely used.
I just posted patches to delete it instead:

    [PATCH 00/12] Error reporting cleanup, a fix, and &error_warn removal

Thank you!