[PATCH 2.5/6] error: Poison g_autoptr(Error) to prevent its use

Markus Armbruster posted 1 patch 2 days, 2 hours ago
Failed in applying to current master (apply log)
include/qapi/error.h | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
[PATCH 2.5/6] error: Poison g_autoptr(Error) to prevent its use
Posted by Markus Armbruster 2 days, 2 hours ago
The previous commit reverted support for g_autoptr(Error).  This one
should stop it from coming back.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 include/qapi/error.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/include/qapi/error.h b/include/qapi/error.h
index f3ce4a4a2d..2356b84bb3 100644
--- a/include/qapi/error.h
+++ b/include/qapi/error.h
@@ -437,6 +437,26 @@ Error *error_copy(const Error *err);
  */
 void error_free(Error *err);
 
+/*
+ * Poison g_autoptr(Error) to prevent its use.
+ *
+ * Functions that report or propagate an error take ownership of the
+ * Error object.  Explicit error_free() is needed when you handle an
+ * error in some other way.  This is rare.
+ *
+ * g_autoptr(Error) would call error_free() automatically on return.
+ * To avoid a double-free, we'd have to manually clear the pointer
+ * every time we propagate or report.
+ *
+ * Thus, g_autoptr(Error) would make the rare case easier to get right
+ * (less prone to leaks), and the common case easier to get wrong
+ * (more prone to double-free).
+ */
+extern void
+__attribute__((error("Do not use g_autoptr() to declare Error * variables")))
+error_free_poisoned(Error *err);
+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free_poisoned)
+
 /*
  * Convenience function to assert that *@errp is set, then silently free it.
  */
-- 
2.49.0
Re: [PATCH 2.5/6] error: Poison g_autoptr(Error) to prevent its use
Posted by Cédric Le Goater 1 day, 18 hours ago
On 11/27/25 08:10, Markus Armbruster wrote:
> The previous commit reverted support for g_autoptr(Error).  This one
> should stop it from coming back.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>   include/qapi/error.h | 20 ++++++++++++++++++++
>   1 file changed, 20 insertions(+)
> 
> diff --git a/include/qapi/error.h b/include/qapi/error.h
> index f3ce4a4a2d..2356b84bb3 100644
> --- a/include/qapi/error.h
> +++ b/include/qapi/error.h
> @@ -437,6 +437,26 @@ Error *error_copy(const Error *err);
>    */
>   void error_free(Error *err);
>   
> +/*
> + * Poison g_autoptr(Error) to prevent its use.
> + *
> + * Functions that report or propagate an error take ownership of the
> + * Error object.  Explicit error_free() is needed when you handle an
> + * error in some other way.  This is rare.
> + *
> + * g_autoptr(Error) would call error_free() automatically on return.
> + * To avoid a double-free, we'd have to manually clear the pointer
> + * every time we propagate or report.
> + *
> + * Thus, g_autoptr(Error) would make the rare case easier to get right
> + * (less prone to leaks), and the common case easier to get wrong
> + * (more prone to double-free).
> + */
> +extern void
> +__attribute__((error("Do not use g_autoptr() to declare Error * variables")))
> +error_free_poisoned(Error *err);
> +G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free_poisoned)
> +
>   /*
>    * Convenience function to assert that *@errp is set, then silently free it.
>    */

broken build, even better.

Tested-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.