| Message ID | 20200707165037.1026246-2-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | error: auto propagated local_err part I | expand |
On 7/7/20 11:50 AM, Markus Armbruster wrote: > From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > > Introduce a new ERRP_AUTO_PROPAGATE macro, to be used at start of > functions with an errp OUT parameter. > > It has three goals: > > 1. Fix issue with error_fatal and error_prepend/error_append_hint: user the 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 and error_propagate: when we wrap > error_abort by local_err+error_propagate, the 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 us to [3.] drop > the 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, later patches will add invocations > of this macro at the start of functions with either use > error_prepend/error_append_hint (solving 1) or which use > local_err+error_propagate to check errors, switching those > functions to use *errp instead (solving 2 and 3). > > Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> > Reviewed-by: Paul Durrant <paul@xen.org> > Reviewed-by: Greg Kurz <groug@kaod.org> > Reviewed-by: Eric Blake <eblake@redhat.com> > [Comments merged properly with recent commit "error: Document Error > API usage rules", and edited for clarity. Put ERRP_AUTO_PROPAGATE() > before its helpers, and touch up style. Commit message tweaked.] > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > include/qapi/error.h | 160 ++++++++++++++++++++++++++++++++++++++----- > 1 file changed, 141 insertions(+), 19 deletions(-) > > diff --git a/include/qapi/error.h b/include/qapi/error.h > index 3fed49747d..c865a7d2f1 100644 > --- a/include/qapi/error.h > +++ b/include/qapi/error.h > @@ -128,18 +122,26 @@ > * handle the error... > * } > * when it doesn't, say a void function: > + * ERRP_AUTO_PROPAGATE(); > + * foo(arg, errp); > + * if (*errp) { > + * handle the error... > + * } > + * More on ERRP_AUTO_PROPAGATE() below. > + * > + * Code predating ERRP_AUTO_PROPAGATE() still exits, and looks like this: exists > * Error *err = NULL; > * foo(arg, &err); > * if (err) { > * handle the error... > - * error_propagate(errp, err); > + * error_propagate(errp, err); // deprecated > * } > - * Do *not* "optimize" this to > + * Avoid in new code. Do *not* "optimize" it to > * foo(arg, errp); > * if (*errp) { // WRONG! > * handle the error... > * } > - * because errp may be NULL! > + * because errp may be NULL! Guard with ERRP_AUTO_PROPAGATE(). maybe: because errp may be NULL without the ERRP_AUTO_PROPAGATE() guard. > * > * But when all you do with the error is pass it on, please use > * foo(arg, errp); > @@ -158,6 +160,19 @@ > * handle the error... > * } > * > + * Pass an existing error to the caller: > + * = Converting to ERRP_AUTO_PROPAGATE() = > + * > + * To convert a function to use ERRP_AUTO_PROPAGATE(): > + * > + * 0. If the Error ** parameter is not named @errp, rename it to > + * @errp. > + * > + * 1. Add an ERRP_AUTO_PROPAGATE() invocation, by convention right at > + * the beginning of the function. This makes @errp safe to use. > + * > + * 2. Replace &err by errp, and err by *errp. Delete local variable > + * @err. > + * > + * 3. Delete error_propagate(errp, *errp), replace > + * error_propagate_prepend(errp, *errp, ...) by error_prepend(errp, ...), > + * Why a comma here? > + * 4. Ensure @errp is valid at return: when you destroy *errp, set > + * errp = NULL. > + * > + * Example: > + * > + * bool fn(..., Error **errp) > + * { > + * Error *err = NULL; > + * > + * foo(arg, &err); > + * if (err) { > + * handle the error... > + * error_propagate(errp, err); > + * return false; > + * } > + * ... > + * } > + * > + * becomes > + * > + * bool fn(..., Error **errp) > + * { > + * ERRP_AUTO_PROPAGATE(); > + * > + * foo(arg, errp); > + * if (*errp) { > + * handle the error... > + * return false; > + * } > + * ... > + * } Do we want the example to show the use of error_free and *errp = NULL? Otherwise, this is looking good to me. It will need a tweak if we go with the shorter name ERRP_GUARD, but I like that idea.
Eric Blake <eblake@redhat.com> writes: > On 7/7/20 11:50 AM, Markus Armbruster wrote: >> From: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> >> Introduce a new ERRP_AUTO_PROPAGATE macro, to be used at start of >> functions with an errp OUT parameter. >> >> It has three goals: >> >> 1. Fix issue with error_fatal and error_prepend/error_append_hint: user > > the user Yes. >> 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 and error_propagate: when we wrap >> error_abort by local_err+error_propagate, the 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 us to [3.] drop >> the 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, later patches will add invocations >> of this macro at the start of functions with either use >> error_prepend/error_append_hint (solving 1) or which use >> local_err+error_propagate to check errors, switching those >> functions to use *errp instead (solving 2 and 3). >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> >> Reviewed-by: Paul Durrant <paul@xen.org> >> Reviewed-by: Greg Kurz <groug@kaod.org> >> Reviewed-by: Eric Blake <eblake@redhat.com> >> [Comments merged properly with recent commit "error: Document Error >> API usage rules", and edited for clarity. Put ERRP_AUTO_PROPAGATE() >> before its helpers, and touch up style. Commit message tweaked.] >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> include/qapi/error.h | 160 ++++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 141 insertions(+), 19 deletions(-) >> >> diff --git a/include/qapi/error.h b/include/qapi/error.h >> index 3fed49747d..c865a7d2f1 100644 >> --- a/include/qapi/error.h >> +++ b/include/qapi/error.h > >> @@ -128,18 +122,26 @@ >> * handle the error... >> * } >> * when it doesn't, say a void function: >> + * ERRP_AUTO_PROPAGATE(); >> + * foo(arg, errp); >> + * if (*errp) { >> + * handle the error... >> + * } >> + * More on ERRP_AUTO_PROPAGATE() below. >> + * >> + * Code predating ERRP_AUTO_PROPAGATE() still exits, and looks like this: > > exists Fixing... >> * Error *err = NULL; >> * foo(arg, &err); >> * if (err) { >> * handle the error... >> - * error_propagate(errp, err); >> + * error_propagate(errp, err); // deprecated >> * } >> - * Do *not* "optimize" this to >> + * Avoid in new code. Do *not* "optimize" it to >> * foo(arg, errp); >> * if (*errp) { // WRONG! >> * handle the error... >> * } >> - * because errp may be NULL! >> + * because errp may be NULL! Guard with ERRP_AUTO_PROPAGATE(). > > maybe: > > because errp may be NULL without the ERRP_AUTO_PROPAGATE() guard. Sold. >> * >> * But when all you do with the error is pass it on, please use >> * foo(arg, errp); >> @@ -158,6 +160,19 @@ >> * handle the error... >> * } >> * >> + * Pass an existing error to the caller: > >> + * = Converting to ERRP_AUTO_PROPAGATE() = >> + * >> + * To convert a function to use ERRP_AUTO_PROPAGATE(): >> + * >> + * 0. If the Error ** parameter is not named @errp, rename it to >> + * @errp. >> + * >> + * 1. Add an ERRP_AUTO_PROPAGATE() invocation, by convention right at >> + * the beginning of the function. This makes @errp safe to use. >> + * >> + * 2. Replace &err by errp, and err by *errp. Delete local variable >> + * @err. >> + * >> + * 3. Delete error_propagate(errp, *errp), replace >> + * error_propagate_prepend(errp, *errp, ...) by error_prepend(errp, ...), >> + * > > Why a comma here? Editing accident. >> + * 4. Ensure @errp is valid at return: when you destroy *errp, set >> + * errp = NULL. >> + * >> + * Example: >> + * >> + * bool fn(..., Error **errp) >> + * { >> + * Error *err = NULL; >> + * >> + * foo(arg, &err); >> + * if (err) { >> + * handle the error... >> + * error_propagate(errp, err); >> + * return false; >> + * } >> + * ... >> + * } >> + * >> + * becomes >> + * >> + * bool fn(..., Error **errp) >> + * { >> + * ERRP_AUTO_PROPAGATE(); >> + * >> + * foo(arg, errp); >> + * if (*errp) { >> + * handle the error... >> + * return false; >> + * } >> + * ... >> + * } > > Do we want the example to show the use of error_free and *errp = NULL? Yes, but we're running out of time, so let's do it in the series that introduces the usage to the code. > Otherwise, this is looking good to me. It will need a tweak if we go > with the shorter name ERRP_GUARD, but I like that idea. Tweaking away... Thanks!
07.07.2020 19:50, Markus Armbruster wrote: > From: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> > > Introduce a new ERRP_AUTO_PROPAGATE macro, to be used at start of > functions with an errp OUT parameter. > > It has three goals: > > 1. Fix issue with error_fatal and 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 and error_propagate: when we wrap > error_abort by local_err+error_propagate, the 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 us to [3.] drop > the 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, later patches will add invocations > of this macro at the start of functions with either use > error_prepend/error_append_hint (solving 1) or which use > local_err+error_propagate to check errors, switching those > functions to use *errp instead (solving 2 and 3). > > Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> > Reviewed-by: Paul Durrant<paul@xen.org> > Reviewed-by: Greg Kurz<groug@kaod.org> > Reviewed-by: Eric Blake<eblake@redhat.com> > [Comments merged properly with recent commit "error: Document Error > API usage rules", and edited for clarity. Put ERRP_AUTO_PROPAGATE() > before its helpers, and touch up style. Commit message tweaked.] > Signed-off-by: Markus Armbruster<armbru@redhat.com> Ok, I see you have mostly rewritten the big comment and not only in this patch, so, I go and read the whole comment on top of these series. ================================= * Pass an existing error to the caller with the message modified: * error_propagate_prepend(errp, err, * "Could not frobnicate '%s': ", name); * This is more concise than * error_propagate(errp, err); // don't do this * error_prepend(errp, "Could not frobnicate '%s': ", name); * and works even when @errp is &error_fatal. - the latter doesn't consider ERRP_AUTO_PROPAGATE: as we know, that ERRP_AUTO_PROPAGATE should be used when we use error_prepend, the latter should look like ERRP_AUTO_PROPAGATE(); ... error_propagate(errp, err); // don't do this error_prepend(errp, "Could not frobnicate '%s': ", name); - and it works even when @errp is &error_fatal, so the error_propagate_prepend now is just a shortcut, not the only correct way. Still, the text is formally correct as is, and may be improved later. ================================= * 2. Replace &err by errp, and err by *errp. Delete local variable * @err. - hmm a bit not obvious,, It can be local_err. It can be (in some rare cases) still needed to handle the error locally, not passing to the caller.. may be just something like "Assume local Error *err variable is used to get errors from called functions and than propagated to caller's errp" before paragraph [2.] will help. * * 3. Delete error_propagate(errp, *errp), replace * error_propagate_prepend(errp, *errp, ...) by error_prepend(errp, ...), * * 4. Ensure @errp is valid at return: when you destroy *errp, set * errp = NULL. ================================= May be good to add note about ERRP_AUTO_PROPAGATE() into comment above error_append_hint (and error_(v)prepend)). ================================= /* * Make @errp parameter easier to use regardless of argument value may be s/argument/its/ * * This macro is for use right at the beginning of a function that * takes an Error **errp parameter to pass errors to its caller. The * parameter must be named @errp. * * It must be used when the function dereferences @errp or passes * @errp to error_prepend(), error_vprepend(), or error_append_hint(). * 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. * * 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. */ ================================= All these are minor, the documentation is good as is, thank you!
Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com> writes: > 07.07.2020 19:50, Markus Armbruster wrote: >> From: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> >> >> Introduce a new ERRP_AUTO_PROPAGATE macro, to be used at start of >> functions with an errp OUT parameter. >> >> It has three goals: >> >> 1. Fix issue with error_fatal and 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 and error_propagate: when we wrap >> error_abort by local_err+error_propagate, the 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 us to [3.] drop >> the 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, later patches will add invocations >> of this macro at the start of functions with either use >> error_prepend/error_append_hint (solving 1) or which use >> local_err+error_propagate to check errors, switching those >> functions to use *errp instead (solving 2 and 3). >> >> Signed-off-by: Vladimir Sementsov-Ogievskiy<vsementsov@virtuozzo.com> >> Reviewed-by: Paul Durrant<paul@xen.org> >> Reviewed-by: Greg Kurz<groug@kaod.org> >> Reviewed-by: Eric Blake<eblake@redhat.com> >> [Comments merged properly with recent commit "error: Document Error >> API usage rules", and edited for clarity. Put ERRP_AUTO_PROPAGATE() >> before its helpers, and touch up style. Commit message tweaked.] >> Signed-off-by: Markus Armbruster<armbru@redhat.com> > > Ok, I see you have mostly rewritten the big comment Guilty as charged... I was happy with the contents you provided (and grateful for it), but our parallel work caused some redundancy. I went beyond a minimal merge to get a something that reads as one coherent text. > and not only in this patch, so, I go and read the whole comment on top of these series. > > ================================= > > * Pass an existing error to the caller with the message modified: > * error_propagate_prepend(errp, err, > * "Could not frobnicate '%s': ", name); > * This is more concise than > * error_propagate(errp, err); // don't do this > * error_prepend(errp, "Could not frobnicate '%s': ", name); > * and works even when @errp is &error_fatal. > > - the latter doesn't consider ERRP_AUTO_PROPAGATE: as we know, that ERRP_AUTO_PROPAGATE should be used when we use error_prepend, the latter should look like > > > ERRP_AUTO_PROPAGATE(); > ... > error_propagate(errp, err); // don't do this > error_prepend(errp, "Could not frobnicate '%s': ", name); > > - and it works even when @errp is &error_fatal, so the error_propagate_prepend now is just a shortcut, not the only correct way. I can duplicate the advice from the paragraph preceding it, like this: * This is rarely needed. When @err is a local variable, use of * ERRP_GUARD() commonly results in more readable code. * Where it is needed, it is more concise than * error_propagate(errp, err); // don't do this * error_prepend(errp, "Could not frobnicate '%s': ", name); * and works even when @errp is &error_fatal. > Still, the text is formally correct as is, and may be improved later. > > ================================= > > * 2. Replace &err by errp, and err by *errp. Delete local variable > * @err. > > - hmm a bit not obvious,, It can be local_err. Yes, but I trust the reader can make that mental jump. > It can be (in some rare cases) still needed to handle the error locally, not passing to the caller.. I didn't think of this. What about * To convert a function to use ERRP_GUARD(), assuming the local * variable it propagates to @errp is called @err: [...] * 2. Replace &err by errp, and err by *errp. Delete local variable * @err if it s now unused. Nope, still no good, if we replace like that, @err *will* be unused, and the locally handled error will leak to the caller. No time left for wordsmithing; let's improve on top. > may be just something like "Assume local Error *err variable is used to get errors from called functions and than propagated to caller's errp" before paragraph [2.] will help. > > > * > * 3. Delete error_propagate(errp, *errp), replace > * error_propagate_prepend(errp, *errp, ...) by error_prepend(errp, ...), > * > * 4. Ensure @errp is valid at return: when you destroy *errp, set > * errp = NULL. > > ================================= > > > May be good to add note about ERRP_AUTO_PROPAGATE() into comment above error_append_hint (and error_(v)prepend)). Good point. > ================================= > > /* > * Make @errp parameter easier to use regardless of argument value > > may be s/argument/its/ > > * > * This macro is for use right at the beginning of a function that > * takes an Error **errp parameter to pass errors to its caller. The > * parameter must be named @errp. > * > * It must be used when the function dereferences @errp or passes > * @errp to error_prepend(), error_vprepend(), or error_append_hint(). > * 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. > * > * 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. > */ > > ================================= > > > All these are minor, the documentation is good as is, thank you! Thanks for your review, and your patience!
diff --git a/include/qapi/error.h b/include/qapi/error.h index 3fed49747d..c865a7d2f1 100644 --- a/include/qapi/error.h +++ b/include/qapi/error.h @@ -30,6 +30,10 @@ * job. Since the value of @errp is about handling the error, the * function should not examine it. * + * - The function may pass @errp to functions it calls to pass on + * their errors to its caller. If it dereferences @errp to check + * for errors, it must use ERRP_AUTO_PROPAGATE(). + * * - On success, the function should not touch *errp. On failure, it * should set a new error, e.g. with error_setg(errp, ...), or * propagate an existing one, e.g. with error_propagate(errp, ...). @@ -45,15 +49,17 @@ * = Creating errors = * * Create an error: - * error_setg(&err, "situation normal, all fouled up"); + * error_setg(errp, "situation normal, all fouled up"); + * where @errp points to the location to receive the error. * * Create an error and add additional explanation: - * error_setg(&err, "invalid quark"); - * error_append_hint(&err, "Valid quarks are up, down, strange, " + * error_setg(errp, "invalid quark"); + * error_append_hint(errp, "Valid quarks are up, down, strange, " * "charm, top, bottom.\n"); + * This may require use of ERRP_AUTO_PROPAGATE(); more on that below. * * Do *not* contract this to - * error_setg(&err, "invalid quark\n" // WRONG! + * error_setg(errp, "invalid quark\n" // WRONG! * "Valid quarks are up, down, strange, charm, top, bottom."); * * = Reporting and destroying errors = @@ -107,18 +113,6 @@ * Errors get passed to the caller through the conventional @errp * parameter. * - * Pass an existing error to the caller: - * error_propagate(errp, err); - * where Error **errp is a parameter, by convention the last one. - * - * Pass an existing error to the caller with the message modified: - * error_propagate_prepend(errp, err, - * "Could not frobnicate '%s': ", name); - * This is more concise than - * error_propagate(errp, err); // don't do this - * error_prepend(errp, "Could not frobnicate '%s': ", name); - * and works even when @errp is &error_fatal. - * * Create a new error and pass it to the caller: * error_setg(errp, "situation normal, all fouled up"); * @@ -128,18 +122,26 @@ * handle the error... * } * when it doesn't, say a void function: + * ERRP_AUTO_PROPAGATE(); + * foo(arg, errp); + * if (*errp) { + * handle the error... + * } + * More on ERRP_AUTO_PROPAGATE() below. + * + * Code predating ERRP_AUTO_PROPAGATE() still exits, and looks like this: * Error *err = NULL; * foo(arg, &err); * if (err) { * handle the error... - * error_propagate(errp, err); + * error_propagate(errp, err); // deprecated * } - * Do *not* "optimize" this to + * Avoid in new code. Do *not* "optimize" it to * foo(arg, errp); * if (*errp) { // WRONG! * handle the error... * } - * because errp may be NULL! + * because errp may be NULL! Guard with ERRP_AUTO_PROPAGATE(). * * But when all you do with the error is pass it on, please use * foo(arg, errp); @@ -158,6 +160,19 @@ * handle the error... * } * + * Pass an existing error to the caller: + * error_propagate(errp, err); + * This is rarely needed. When @err is a local variable, use of + * ERRP_AUTO_PROPAGATE() commonly results in more readable code. + * + * Pass an existing error to the caller with the message modified: + * error_propagate_prepend(errp, err, + * "Could not frobnicate '%s': ", name); + * This is more concise than + * error_propagate(errp, err); // don't do this + * error_prepend(errp, "Could not frobnicate '%s': ", name); + * and works even when @errp is &error_fatal. + * * Receive and accumulate multiple errors (first one wins): * Error *err = NULL, *local_err = NULL; * foo(arg, &err); @@ -185,6 +200,70 @@ * error_setg(&err, ...); // WRONG! * } * because this may pass a non-null err to error_setg(). + * + * = Why, when and how to use ERRP_AUTO_PROPAGATE() = + * + * Without ERRP_AUTO_PROPAGATE(), use of the @errp parameter is + * restricted: + * - It must not be dereferenced, because it may be null. + * - It should not be passed to error_prepend() or + * error_append_hint(), because that doesn't work with &error_fatal. + * ERRP_AUTO_PROPAGATE() lifts these restrictions. + * + * To use ERRP_AUTO_PROPAGATE(), add it right at the beginning of the + * function. @errp can then be used without worrying about the + * argument being NULL or &error_fatal. + * + * Using it when it's not needed is safe, but please avoid cluttering + * the source with useless code. + * + * = Converting to ERRP_AUTO_PROPAGATE() = + * + * To convert a function to use ERRP_AUTO_PROPAGATE(): + * + * 0. If the Error ** parameter is not named @errp, rename it to + * @errp. + * + * 1. Add an ERRP_AUTO_PROPAGATE() invocation, by convention right at + * the beginning of the function. This makes @errp safe to use. + * + * 2. Replace &err by errp, and err by *errp. Delete local variable + * @err. + * + * 3. Delete error_propagate(errp, *errp), replace + * error_propagate_prepend(errp, *errp, ...) by error_prepend(errp, ...), + * + * 4. Ensure @errp is valid at return: when you destroy *errp, set + * errp = NULL. + * + * Example: + * + * bool fn(..., Error **errp) + * { + * Error *err = NULL; + * + * foo(arg, &err); + * if (err) { + * handle the error... + * error_propagate(errp, err); + * return false; + * } + * ... + * } + * + * becomes + * + * bool fn(..., Error **errp) + * { + * ERRP_AUTO_PROPAGATE(); + * + * foo(arg, errp); + * if (*errp) { + * handle the error... + * return false; + * } + * ... + * } */ #ifndef ERROR_H @@ -285,6 +364,7 @@ void error_setg_win32_internal(Error **errp, * the error object. * Else, move the error object from @local_err to *@dst_errp. * On return, @local_err is invalid. + * Please use ERRP_AUTO_PROPAGATE() instead when possible. * Please don't error_propagate(&error_fatal, ...), use * error_report_err() and exit(), because that's more obvious. */ @@ -296,6 +376,8 @@ void error_propagate(Error **dst_errp, Error *local_err); * Behaves like * error_prepend(&local_err, fmt, ...); * error_propagate(dst_errp, local_err); + * Please use ERRP_AUTO_PROPAGATE() and error_prepend() instead when + * possible. */ void error_propagate_prepend(Error **dst_errp, Error *local_err, const char *fmt, ...); @@ -393,6 +475,46 @@ void error_set_internal(Error **errp, ErrorClass err_class, const char *fmt, ...) GCC_FMT_ATTR(6, 7); +/* + * Make @errp parameter easier to use regardless of argument value + * + * This macro is for use right at the beginning of a function that + * takes an Error **errp parameter to pass errors to its caller. The + * parameter must be named @errp. + * + * It must be used when the function dereferences @errp or passes + * @errp to error_prepend(), error_vprepend(), or error_append_hint(). + * 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. + * + * 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_AUTO_PROPAGATE() \ + g_auto(ErrorPropagator) _auto_errp_prop = {.errp = errp}; \ + do { \ + if (!errp || errp == &error_fatal) { \ + errp = &_auto_errp_prop.local_err; \ + } \ + } while (0) + +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); + /* * Special error destination to abort on error. * See error_setg() and error_propagate() for details.