[v12,1/8] error: New macro ERRP_AUTO_PROPAGATE()
diff mbox series

Message ID 20200707165037.1026246-2-armbru@redhat.com
State New
Headers show
Series
  • error: auto propagated local_err part I
Related show

Commit Message

Markus Armbruster July 7, 2020, 4:50 p.m. UTC
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>
---
 include/qapi/error.h | 160 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 141 insertions(+), 19 deletions(-)

Comments

Eric Blake July 7, 2020, 7:02 p.m. UTC | #1
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.
Markus Armbruster July 7, 2020, 8:01 p.m. UTC | #2
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!
Vladimir Sementsov-Ogievskiy July 7, 2020, 8:08 p.m. UTC | #3
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!
Markus Armbruster July 7, 2020, 8:43 p.m. UTC | #4
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!

Patch
diff mbox series

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.