[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
Re: [PATCH 20/21] hw/intc/s390: Simplify error handling in kvm_s390_flic_realize()
Wed, 04 Dec 2019 08:28:13 +0100
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>
> 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).
> 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 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". If you do it anyway, it
logs a warning nobody reads, then throws away the newer error.
 It's "ex post facto" documentation, like much of the Error API
 First order approximation based on the amount of crap supposedly
stable applications log.