[v1] hw: Replace some impossible checks by assertions

[PATCH 5/5] hw/isa/isa-bus: Replace hw_error() by assert()

Posted by Philippe Mathieu-Daudé 5 years, 5 months ago

As we can never have more than ISA_NUM_IRQS (16) ISA IRQs,
replace the not very interesting hw_error() call by an
assert() which is more useful to debug condition that can
not happen.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/isa/isa-bus.c | 9 ++-------
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
index 58fde178f92..10bb7ffa43a 100644
--- a/hw/isa/isa-bus.c
+++ b/hw/isa/isa-bus.c
@@ -21,7 +21,6 @@
 #include "qemu/error-report.h"
 #include "qemu/module.h"
 #include "qapi/error.h"
-#include "hw/hw.h"
 #include "monitor/monitor.h"
 #include "hw/sysbus.h"
 #include "sysemu/sysemu.h"
@@ -85,18 +84,14 @@ void isa_bus_irqs(ISABus *bus, qemu_irq *irqs)
 qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq)
 {
     assert(!dev || ISA_BUS(qdev_get_parent_bus(DEVICE(dev))) == isabus);
-    if (isairq >= ISA_NUM_IRQS) {
-        hw_error("isa irq %d invalid", isairq);
-    }
+    assert(isairq < ISA_NUM_IRQS);
     return isabus->irqs[isairq];
 }
 
 void isa_init_irq(ISADevice *dev, qemu_irq *p, unsigned isairq)
 {
     assert(dev->nirqs < ARRAY_SIZE(dev->isairq));
-    if (isairq >= ISA_NUM_IRQS) {
-        hw_error("isa irq %d invalid", isairq);
-    }
+    assert(isairq < ISA_NUM_IRQS);
     dev->isairq[dev->nirqs] = isairq;
     *p = isa_get_irq(dev, isairq);
     dev->nirqs++;
-- 
2.26.2

Re: [PATCH 5/5] hw/isa/isa-bus: Replace hw_error() by assert()

Posted by Richard Henderson 5 years, 5 months ago

On 9/1/20 3:40 AM, Philippe Mathieu-Daudé wrote:
> As we can never have more than ISA_NUM_IRQS (16) ISA IRQs,
> replace the not very interesting hw_error() call by an
> assert() which is more useful to debug condition that can
> not happen.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/isa/isa-bus.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>

r~

Re: [PATCH 5/5] hw/isa/isa-bus: Replace hw_error() by assert()

Posted by Laurent Vivier 5 years, 5 months ago

Le 01/09/2020 à 12:40, Philippe Mathieu-Daudé a écrit :
> As we can never have more than ISA_NUM_IRQS (16) ISA IRQs,
> replace the not very interesting hw_error() call by an
> assert() which is more useful to debug condition that can
> not happen.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/isa/isa-bus.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/hw/isa/isa-bus.c b/hw/isa/isa-bus.c
> index 58fde178f92..10bb7ffa43a 100644
> --- a/hw/isa/isa-bus.c
> +++ b/hw/isa/isa-bus.c
> @@ -21,7 +21,6 @@
>  #include "qemu/error-report.h"
>  #include "qemu/module.h"
>  #include "qapi/error.h"
> -#include "hw/hw.h"
>  #include "monitor/monitor.h"
>  #include "hw/sysbus.h"
>  #include "sysemu/sysemu.h"
> @@ -85,18 +84,14 @@ void isa_bus_irqs(ISABus *bus, qemu_irq *irqs)
>  qemu_irq isa_get_irq(ISADevice *dev, unsigned isairq)
>  {
>      assert(!dev || ISA_BUS(qdev_get_parent_bus(DEVICE(dev))) == isabus);
> -    if (isairq >= ISA_NUM_IRQS) {
> -        hw_error("isa irq %d invalid", isairq);
> -    }
> +    assert(isairq < ISA_NUM_IRQS);
>      return isabus->irqs[isairq];
>  }
>  
>  void isa_init_irq(ISADevice *dev, qemu_irq *p, unsigned isairq)
>  {
>      assert(dev->nirqs < ARRAY_SIZE(dev->isairq));
> -    if (isairq >= ISA_NUM_IRQS) {
> -        hw_error("isa irq %d invalid", isairq);
> -    }
> +    assert(isairq < ISA_NUM_IRQS);
>      dev->isairq[dev->nirqs] = isairq;
>      *p = isa_get_irq(dev, isairq);
>      dev->nirqs++;
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

[PATCH 1/5] hw/gpio/max7310: Remove impossible check
[PATCH 2/5] hw/mips/fuloong2e: Convert pointless error message to an assert()
[PATCH 3/5] hw/ppc/ppc4xx_pci: Use ARRAY_SIZE() instead of magic value
[PATCH 4/5] hw/ppc/ppc4xx_pci: Replace pointless warning by assert()
[PATCH 5/5] hw/isa/isa-bus: Replace hw_error() by assert()