Re: [PULL v2 05/25] error: add global &error_warn destination

[Peter Maydell]

On Thu, 6 Apr 2023 at 15:13, Stefan Berger <stefanb@linux.ibm.com> wrote:
> I'll be out starting tomorrow. I don't see Marc-André online.
>
> Would this be acceptable?
> It ensures that if error_handle() returns, err has been freed.
> In the other two cases a copy is being made of the Error that can then be 
> used after the error_handle() call.

"Not error_warn" is the common case, so it doesn't seem
great to copy the error around like that. My thoughts were
either:
 (1) error_handle() should handle all of the error cases,
like this:

    if (errp == &error_abort) {
       ...
       abort();
    }
    if (errp == &error_fatal) {
       ...
       exit(1);
    }
    if (errp = &error_warn) {
        warn_report_err(err);
    } else if (errp && !*errp) {
        *errp = err;
    } else {
        error_free(err);
    }

and delete the "set *errp" logic from the callers.

(note that error_setv() has done checks already that mean it
will always take the "(errp && !*errp)" path, so we don't need
to special-case for which caller this was.)

(2) error_handle() should return a bool to say whether it's handled
the error entirely and the callsite should do nothing further
with it.

I prefer (1) I think but would defer to people with more
experience with the Error APIs.

thanks
-- PMM