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: Richard Henderson
Subject: Re: [Qemu-devel] [PATCH v2 11/13] isa: Clean up inappropriate hw_error()
Date: Thu, 17 Dec 2015 08:38:24 -0800
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.4.0

On 12/17/2015 06:27 AM, Markus Armbruster wrote:
"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:

I'm not a fan of sprinkling obvious assertions everywhere either. I think the patch is fine as-is.

Reviewed-by: Richard Henderson <address@hidden>


r~



reply via email to

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