[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 20/21] hw/intc/s390: Simplify error handling in kvm_s390_flic
From: |
Markus Armbruster |
Subject: |
Re: [PATCH 20/21] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize() |
Date: |
Wed, 04 Dec 2019 08:28:13 +0100 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) |
Halil Pasic <address@hidden> writes:
> On Mon, 2 Dec 2019 17:40:07 +0100
> Cornelia Huck <address@hidden> wrote:
>
>> On Sat, 30 Nov 2019 20:42:39 +0100
>> Markus Armbruster <address@hidden> wrote:
>>
>> > Cc: Halil Pasic <address@hidden>
>> > Cc: Cornelia Huck <address@hidden>
>> > Cc: Christian Borntraeger <address@hidden>
>> > Signed-off-by: Markus Armbruster <address@hidden>
>> > ---
>> > hw/intc/s390_flic_kvm.c | 10 ++++------
>> > 1 file changed, 4 insertions(+), 6 deletions(-)
>>
>> Reviewed-by: Cornelia Huck <address@hidden>
>>
>> ...someone else wants to take a look before I queue this?
>>
>
> I guess it is a matter of taste.
>
> Acked-by: Halil Pasic <address@hidden>
Thanks!
> The only difference I can see is if the **errp argument where
> to already contain an error (*errp). I guess the old code would
> just keep the first error, and discard the next one, while error_setv()
> does an assert(*errp == NULL).
Correct.
> I assume calling with *errp != NULL is not a valid scenario.
Correct again. On function entry, @errp must either be null,
@error_fatal, @error_abort, or point to null.
> But then, I
> can't say I understand the use-case behind this discard the newer error
> behavior of error_propagate().
The documented[1] use case is "receive and accumulate multiple errors
(first one wins)". See the big comment in include/qapi/error.h.
For what it's worth, GError explicitly disallows such accumulation: "The
error variable dest points to must be NULL"[2]. If you do it anyway, it
logs a warning nobody reads[3], then throws away the newer error.
[1] It's "ex post facto" documentation, like much of the Error API
documentation.
[2]
https://developer.gnome.org/glib/stable/glib-Error-Reporting.html#g-propagate-error
[3] First order approximation based on the amount of crap supposedly
stable applications log.