Re: [PATCH 10/16] qapi/error: add g_autoptr(Error) support

From: Paolo Bonzini <pbonzini@redhat.com>
To: marcandre.lureau@redhat.com, qemu-devel@nongnu.org
Cc: "Thomas Huth" <thuth@redhat.com>, "Stefan Weil" <sw@weilnetz.de>,
	"Qiuhao Li" <Qiuhao.Li@outlook.com>,
	"Laurent Vivier" <lvivier@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Hanna Reitz" <hreitz@redhat.com>,
	"Kevin Wolf" <kwolf@redhat.com>,
	"Markus Armbruster" <armbru@redhat.com>,
	qemu-block@nongnu.org, "Konstantin Kostiuk" <kkostiuk@redhat.com>,
	"Bandan Das" <bsd@redhat.com>,
	"Michael Roth" <michael.roth@amd.com>,
	"Darren Kenny" <darren.kenny@oracle.com>,
	"Alexander Bulekov" <alxndr@bu.edu>,
	"Marc-André Lureau" <marcandre.lureau@redhat.com>
Subject: Re: [PATCH 10/16] qapi/error: add g_autoptr(Error) support
Date: Thu, 05 May 2022 08:53:17 +0200	[thread overview]
Message-ID: <DB8BDCA2-868D-46CF-9AF1-28F619B98812@redhat.com> (raw)
In-Reply-To: <20220504173025.650167-11-marcandre.lureau@redhat.com>

This was rejected before on the grounds that propagating or printing the error is usually the right thing to do, and neither needs an autoptr:

https://patchew.org/QEMU/20210912124834.503032-1-pbonzini@redhat.com/

So while I do agree with the patch, for it to be accepted some Error* functions would have to be changed to accept Error** instead. This way they can NULL the variable containing their argument, and prevent the auto-error_free.

Paolo

Il 4 maggio 2022 19:30:19 CEST, marcandre.lureau@redhat.com ha scritto:
>From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
>Sometime, ERRP_GUARD() isn't what you are looking for, because the
>function doesn't throw errors, yet auto-cleaning is nice to have.
>
>Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>---
> include/qapi/error.h | 2 ++
> 1 file changed, 2 insertions(+)
>
>diff --git a/include/qapi/error.h b/include/qapi/error.h
>index d798faeec3e9..9482b6a58ae6 100644
>--- a/include/qapi/error.h
>+++ b/include/qapi/error.h
>@@ -519,6 +519,8 @@ static inline void error_propagator_cleanup(ErrorPropagator *prop)
> 
> G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup);
> 
>+G_DEFINE_AUTOPTR_CLEANUP_FUNC(Error, error_free);
>+
> /*
>  * Special error destination to abort on error.
>  * See error_setg() and error_propagate() for details.