[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?
- [Qemu-devel] [PATCH v2 09/13] isa: Trivially convert remaining PCI-ISA bridges to realize(), (continued)
- [Qemu-devel] [PATCH v2 09/13] isa: Trivially convert remaining PCI-ISA bridges to realize(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 12/13] audio: Clean up inappropriate and unreachable use of hw_error(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 02/13] omap: Don't use hw_error() in device init() methods, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 04/13] etraxfs_eth: Don't use hw_error() in init() method, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 11/13] isa: Clean up inappropriate hw_error(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 10/13] isa: Clean up error handling around isa_bus_new(), Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 08/13] sysbus: Don't use hw_error() in machine_init_done_notifiers, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 13/13] xen-hvm: Mark inappropriate error handling FIXME, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 03/13] arm_mptimer: Don't use hw_error() in realize() method, Markus Armbruster, 2015/12/17
- [Qemu-devel] [PATCH v2 01/13] hw: Don't use hw_error() for machine initialization errors, Markus Armbruster, 2015/12/17