Re: [PATCH] qom: use ERRP_GUARD in user_creatable_complete

[Markus Armbruster <armbru@redhat.com>]

Daniel P. Berrangé <berrange@redhat.com> writes:

> On Fri, Sep 19, 2025 at 01:30:18PM +0200, Markus Armbruster wrote:
>> Daniel P. Berrangé <berrange@redhat.com> writes:
>> 
>> > With error_propagate, the stack trace from any error_abort/fatal
>> > usage will start from the error_propagate() call, which is largely
>> > useless. Using ERRP_GUARD ensures the stack trace starts from
>> > the origin that reported the error.
>> 
>> Yes.
>> 
>> I've been chipping at error_propagate() uses on and off for a while.
>> There are hundreds left.
>
> Are there cases where it is still OK to use error_propagate or should
> we be looking to eliminate all its usage ?

The common use of error_propagate() is to propagate an error received
from a function to the caller.  This is better done with ERRP_GUARD().
qapi/error.h:

 * Call a function, receive an error from it, and pass it to the caller
 * - when the function returns a value that indicates failure, say
 *   false:
 *     if (!foo(arg, errp)) {
 *         handle the error...
 *     }
 * - when it does not, say because it is a void function:
 *     ERRP_GUARD();
 *     foo(arg, errp);
 *     if (*errp) {
 *         handle the error...
 *     }
 * More on ERRP_GUARD() below.
 *
 * Code predating ERRP_GUARD() still exists, and looks like this:
 *     Error *err = NULL;
 *     foo(arg, &err);
 *     if (err) {
 *         handle the error...
 *         error_propagate(errp, err); // deprecated
 *     }

We occasionally store errors on the heap, and use error_propagate() to move
them into an @errp argument.  qapi/error.h:

 * Pass an existing error to the caller:
 *     error_propagate(errp, err);
 * This is rarely needed.  When @err is a local variable, use of
 * ERRP_GUARD() commonly results in more readable code.

error_propagate() can also be used to accumulate errors.  This cannot be
done with ERRP_GUARD().  qapi/error.h:

 * Receive and accumulate multiple errors (first one wins):
 *     Error *err = NULL, *local_err = NULL;
 *     foo(arg, &err);
 *     bar(arg, &local_err);
 *     error_propagate(&err, local_err);
 *     if (err) {
 *         handle the error...
 *     }

Accumulating errors is commonly a bad idea.  Note that
g_propagate_error() explicitly prohibits such usage.  We deviated from
it.

The *possibility* of intentional error acculumation makes conversions to
ERRP_GUARD() harder.  Perhaps we should track down all uses of error
accumulation, then change error_propagate() to prohibit it.

[...]