qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH v2 11/13] isa: Clean up inappropriate hw_error()


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH v2 11/13] isa: Clean up inappropriate hw_error()
Date: Thu, 17 Dec 2015 15:27:36 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

"Michael S. Tsirkin" <address@hidden> writes:

> On Thu, Dec 17, 2015 at 01:19:53PM +0100, Markus Armbruster wrote:
>> isa_bus_irqs(), isa_create() and isa_try_create() call hw_error() when
>> passed a null bus.  Use of hw_error() has always been questionable,
>> because these are used only during machine initialization, and
>> printing CPU registers isn't useful there.
>> 
>> Since the previous commit, passing a null bus is a programming error.
>> Drop the hw_error() and simply let it crash.
>> 
>> Cc: Richard Henderson <address@hidden>
>> Cc: "Michael S. Tsirkin" <address@hidden>
>> Cc: "Hervé Poussineau" <address@hidden>
>> Cc: Aurelien Jarno <address@hidden>
>> Cc: Mark Cave-Ayland <address@hidden>
>> Signed-off-by: Markus Armbruster <address@hidden>
>> Reviewed-by: Hervé Poussineau <address@hidden>
>
> I'd prefer an assert just in case.

I understand "prefer", I don't understand "just in case" :)

Adding an assertion here merely converts one kind of crash into another.

Doesn't make anything safer, not even just in case something happens we
thought was impossible.

Does print a message before crashing that some developers may find
useful.

Might make our belief that null can't happen a bit more explicit.

My own preference is not to assert the blatantly obvious.  However, I'm
certainly willing to defer to a maintainer's or reviewer's preference,
within reason.  For what it's worth:

    $ scripts/get_maintainer.pl -f hw/isa/isa-bus.c 
    get_maintainer.pl: No maintainers found, printing recent contributors.
    get_maintainer.pl: Do not blindly cc: them on patches!  Use common sense.

    "Hervé Poussineau" <address@hidden> (commit_signer:4/6=67%)
    Markus Armbruster <address@hidden> (commit_signer:3/6=50%)
    Leon Alrae <address@hidden> (commit_signer:2/6=33%)
    Paolo Bonzini <address@hidden> (commit_signer:2/6=33%)
    Dave Airlie <address@hidden> (commit_signer:1/6=17%)

Considering all of the above, do you want me to add the assertions?



reply via email to

[Prev in Thread] Current Thread [Next in Thread]