qemu-devel
[Top][All Lists]
Advanced

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

Re: [Qemu-devel] [PATCH 10/12] isa: Clean up inappropriate hw_error()


From: Markus Armbruster
Subject: Re: [Qemu-devel] [PATCH 10/12] isa: Clean up inappropriate hw_error()
Date: Thu, 10 Dec 2015 14:09:00 +0100
User-agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (gnu/linux)

Marcel Apfelbaum <address@hidden> writes:

> On 12/10/2015 12:29 PM, 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.
>
> Maybe we can be a little nicer add an assert ? :)

assert(p) before dereferencing p only converts one kind of crash into
another one.  I tend to do it only when the assert(p) does double-duty
as useful documentation.  Or perhaps when I think there's a real risk of
running into !p in an environment where core dumps are off[*] and
reproducing the failure with a debugger attached could be hard.

To use these three functions, you need an ISABus *.  How could you end
up with a bad one?

* You forget to create the ISA bus, and the compiler is too confused to
  notice.  You'll pass an unitialized ISABus, and asserting it's not
  null is unlikely to help.

* You create multiple ISA buses (that's the only way creating one can
  fail) *and* forget to check for errors.  If you pull that off, I'd
  expect it to explode even in light testing.

* Your pointer gets corrupted between correct initialization and use.
  Asserting it's not null is unlikely to help.


[*] Switching them off on a development machine forfeits your
developer's license, as far as I'm concerned :)



reply via email to

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