Here is introduced ERRP_AUTO_PROPAGATE macro, to be used at start of functions with errp OUT parameter.
It has three goals: 1. Fix issue with error_fatal & error_prepend/error_append_hint: user can't see this additional information, because exit() happens in error_setg earlier than information is added. [Reported by Greg Kurz] 2. Fix issue with error_abort & error_propagate: when we wrap error_abort by local_err+error_propagate, resulting coredump will refer to error_propagate and not to the place where error happened. (the macro itself doesn't fix the issue, but it allows to [3.] drop all local_err+error_propagate pattern, which will definitely fix the issue) [Reported by Kevin Wolf] 3. Drop local_err+error_propagate pattern, which is used to workaround void functions with errp parameter, when caller wants to know resulting status. (Note: actually these functions could be merely updated to return int error code). To achieve these goals, we need to add invocation of the macro at start of functions, which needs error_prepend/error_append_hint (1.); add invocation of the macro at start of functions which do local_err+error_propagate scenario the check errors, drop local errors from them and just use *errp instead (2., 3.). Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> Reviewed-by: Eric Blake <[email protected]> --- CC: Gerd Hoffmann <[email protected]> CC: "Gonglei (Arei)" <[email protected]> CC: Eduardo Habkost <[email protected]> CC: Igor Mammedov <[email protected]> CC: Laurent Vivier <[email protected]> CC: Amit Shah <[email protected]> CC: Kevin Wolf <[email protected]> CC: Max Reitz <[email protected]> CC: John Snow <[email protected]> CC: Ari Sundholm <[email protected]> CC: Pavel Dovgalyuk <[email protected]> CC: Paolo Bonzini <[email protected]> CC: Stefan Hajnoczi <[email protected]> CC: Fam Zheng <[email protected]> CC: Stefan Weil <[email protected]> CC: Ronnie Sahlberg <[email protected]> CC: Peter Lieven <[email protected]> CC: Eric Blake <[email protected]> CC: "Denis V. Lunev" <[email protected]> CC: Markus Armbruster <[email protected]> CC: Alberto Garcia <[email protected]> CC: Jason Dillaman <[email protected]> CC: Wen Congyang <[email protected]> CC: Xie Changlong <[email protected]> CC: Liu Yuan <[email protected]> CC: "Richard W.M. Jones" <[email protected]> CC: Jeff Cody <[email protected]> CC: "Marc-André Lureau" <[email protected]> CC: "Daniel P. Berrangé" <[email protected]> CC: Richard Henderson <[email protected]> CC: Greg Kurz <[email protected]> CC: "Michael S. Tsirkin" <[email protected]> CC: Marcel Apfelbaum <[email protected]> CC: Beniamino Galvani <[email protected]> CC: Peter Maydell <[email protected]> CC: "Cédric Le Goater" <[email protected]> CC: Andrew Jeffery <[email protected]> CC: Joel Stanley <[email protected]> CC: Andrew Baumann <[email protected]> CC: "Philippe Mathieu-Daudé" <[email protected]> CC: Antony Pavlov <[email protected]> CC: Jean-Christophe Dubois <[email protected]> CC: Peter Chubb <[email protected]> CC: Subbaraya Sundeep <[email protected]> CC: Eric Auger <[email protected]> CC: Alistair Francis <[email protected]> CC: "Edgar E. Iglesias" <[email protected]> CC: Stefano Stabellini <[email protected]> CC: Anthony Perard <[email protected]> CC: Paul Durrant <[email protected]> CC: Paul Burton <[email protected]> CC: Aleksandar Rikalo <[email protected]> CC: Chris Wulff <[email protected]> CC: Marek Vasut <[email protected]> CC: David Gibson <[email protected]> CC: Cornelia Huck <[email protected]> CC: Halil Pasic <[email protected]> CC: Christian Borntraeger <[email protected]> CC: "Hervé Poussineau" <[email protected]> CC: Xiao Guangrong <[email protected]> CC: Aurelien Jarno <[email protected]> CC: Aleksandar Markovic <[email protected]> CC: Mark Cave-Ayland <[email protected]> CC: Jason Wang <[email protected]> CC: Laszlo Ersek <[email protected]> CC: Yuval Shaia <[email protected]> CC: Palmer Dabbelt <[email protected]> CC: Sagar Karandikar <[email protected]> CC: Bastian Koppelmann <[email protected]> CC: David Hildenbrand <[email protected]> CC: Thomas Huth <[email protected]> CC: Eric Farman <[email protected]> CC: Matthew Rosato <[email protected]> CC: Hannes Reinecke <[email protected]> CC: Michael Walle <[email protected]> CC: Artyom Tarasenko <[email protected]> CC: Stefan Berger <[email protected]> CC: Samuel Thibault <[email protected]> CC: Alex Williamson <[email protected]> CC: Tony Krowiak <[email protected]> CC: Pierre Morel <[email protected]> CC: Michael Roth <[email protected]> CC: Hailiang Zhang <[email protected]> CC: Juan Quintela <[email protected]> CC: "Dr. David Alan Gilbert" <[email protected]> CC: Luigi Rizzo <[email protected]> CC: Giuseppe Lettieri <[email protected]> CC: Vincenzo Maffione <[email protected]> CC: Jan Kiszka <[email protected]> CC: Anthony Green <[email protected]> CC: Stafford Horne <[email protected]> CC: Guan Xuetao <[email protected]> CC: Max Filippov <[email protected]> CC: [email protected] CC: [email protected] CC: [email protected] CC: [email protected] CC: [email protected] CC: [email protected] CC: [email protected] CC: [email protected] include/qapi/error.h | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/include/qapi/error.h b/include/qapi/error.h index d6898d833b..47238d9065 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -345,6 +345,44 @@ void error_set_internal(Error **errp, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(6, 7); +typedef struct ErrorPropagator { + Error *local_err; + Error **errp; +} ErrorPropagator; + +static inline void error_propagator_cleanup(ErrorPropagator *prop) +{ + error_propagate(prop->errp, prop->local_err); +} + +G_DEFINE_AUTO_CLEANUP_CLEAR_FUNC(ErrorPropagator, error_propagator_cleanup); + +/* + * ERRP_AUTO_PROPAGATE + * + * This macro is created to be the first line of a function with Error **errp + * OUT parameter. It's needed only in cases where we want to use error_prepend, + * error_append_hint or dereference *errp. It's still safe (but useless) in + * other cases. + * + * If errp is NULL or points to error_fatal, it is rewritten to point to a + * local Error object, which will be automatically propagated to the original + * errp on function exit (see error_propagator_cleanup). + * + * After invocation of this macro it is always safe to dereference errp + * (as it's not NULL anymore) and to add information (by error_prepend or + * error_append_hint) + * (as, if it was error_fatal, we swapped it with a local_error to be + * propagated on cleanup). + * + * Note: we don't wrap the error_abort case, as we want resulting coredump + * to point to the place where the error happened, not to error_propagate. + */ +#define ERRP_AUTO_PROPAGATE() \ + g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp}; \ + errp = ((errp == NULL || *errp == error_fatal) \ + ? &_auto_errp_prop.local_err : errp) + /* * Special error destination to abort on error. * See error_setg() and error_propagate() for details. -- 2.21.0
