[PATCH v2 0/2] hw: Replace some impossible checks by assertions

[PATCH v2 0/2] hw: Replace some impossible checks by assertions

[Philippe Mathieu-Daudé]

Trivial patches removing unreachable code.

Since v1:
- dropped patches queued
- replace dead code by assert (Peter)
- use PCI_NUM_PINS definition (Cédric)

Philippe Mathieu-Daudé (2):
  hw/gpio/max7310: Remove impossible check
  hw/ppc/ppc4xx_pci: Replace magic value by the PCI_NUM_PINS definition

 hw/gpio/max7310.c   | 5 +----
 hw/ppc/ppc4xx_pci.c | 2 +-
 2 files changed, 2 insertions(+), 5 deletions(-)

-- 
2.26.2

[PATCH v2 1/2] hw/gpio/max7310: Remove impossible check

[Philippe Mathieu-Daudé]

The max7310_gpio_set() handler is static and only used by
qdev_init_gpio_in, initialized with 8 IRQs. The 'line'
argument can not be out of the [0-8[ range.
Replace the dead code by an assertion.

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/gpio/max7310.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/hw/gpio/max7310.c b/hw/gpio/max7310.c
index 4f78774dc8f..158b0a074e5 100644
--- a/hw/gpio/max7310.c
+++ b/hw/gpio/max7310.c
@@ -8,9 +8,7 @@
  */
 
 #include "qemu/osdep.h"
-#include "hw/hw.h"
 #include "hw/i2c/i2c.h"
-#include "hw/hw.h"
 #include "hw/irq.h"
 #include "migration/vmstate.h"
 #include "qemu/module.h"
@@ -173,8 +171,7 @@ static const VMStateDescription vmstate_max7310 = {
 static void max7310_gpio_set(void *opaque, int line, int level)
 {
     MAX7310State *s = (MAX7310State *) opaque;
-    if (line >= ARRAY_SIZE(s->handler) || line  < 0)
-        hw_error("bad GPIO line");
+    assert(line >= 0 && line < ARRAY_SIZE(s->handler));
 
     if (level)
         s->level |= s->direction & (1 << line);
-- 
2.26.2

[PATCH v2 2/2] hw/ppc/ppc4xx_pci: Replace magic value by the PCI_NUM_PINS definition

[Philippe Mathieu-Daudé]

Replace the magic '4' value by the PCI_NUM_PINS definition.

Suggested-by: Cédric Le Goater <clg@kaod.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/ppc/ppc4xx_pci.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
index 503ef46b39a..930be78361d 100644
--- a/hw/ppc/ppc4xx_pci.c
+++ b/hw/ppc/ppc4xx_pci.c
@@ -54,7 +54,7 @@ struct PPC4xxPCIState {
 
     struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
     struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
-    qemu_irq irq[4];
+    qemu_irq irq[PCI_NUM_PINS];
 
     MemoryRegion container;
     MemoryRegion iomem;
-- 
2.26.2

Re: [PATCH v2 2/2] hw/ppc/ppc4xx_pci: Replace magic value by the PCI_NUM_PINS definition

[David Gibson]

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

On Thu, Sep 10, 2020 at 09:23:25AM +0200, Philippe Mathieu-Daudé wrote:
> Replace the magic '4' value by the PCI_NUM_PINS definition.
> 
> Suggested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/ppc/ppc4xx_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index 503ef46b39a..930be78361d 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -54,7 +54,7 @@ struct PPC4xxPCIState {
>  
>      struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
>      struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
> -    qemu_irq irq[4];
> +    qemu_irq irq[PCI_NUM_PINS];
>  
>      MemoryRegion container;
>      MemoryRegion iomem;

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

Re: [PATCH v2 0/2] hw: Replace some impossible checks by assertions

[Richard Henderson]

On 9/10/20 12:23 AM, Philippe Mathieu-Daudé wrote:
> Trivial patches removing unreachable code.
> 
> Since v1:
> - dropped patches queued
> - replace dead code by assert (Peter)
> - use PCI_NUM_PINS definition (Cédric)
> 
> Philippe Mathieu-Daudé (2):
>   hw/gpio/max7310: Remove impossible check
>   hw/ppc/ppc4xx_pci: Replace magic value by the PCI_NUM_PINS definition
> 
>  hw/gpio/max7310.c   | 5 +----
>  hw/ppc/ppc4xx_pci.c | 2 +-
>  2 files changed, 2 insertions(+), 5 deletions(-)
> 

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


r~

Re: [PATCH v2 1/2] hw/gpio/max7310: Remove impossible check

[Laurent Vivier]

Le 10/09/2020 à 09:23, Philippe Mathieu-Daudé a écrit :
> The max7310_gpio_set() handler is static and only used by
> qdev_init_gpio_in, initialized with 8 IRQs. The 'line'
> argument can not be out of the [0-8[ range.
> Replace the dead code by an assertion.
> 
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/gpio/max7310.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/hw/gpio/max7310.c b/hw/gpio/max7310.c
> index 4f78774dc8f..158b0a074e5 100644
> --- a/hw/gpio/max7310.c
> +++ b/hw/gpio/max7310.c
> @@ -8,9 +8,7 @@
>   */
>  
>  #include "qemu/osdep.h"
> -#include "hw/hw.h"
>  #include "hw/i2c/i2c.h"
> -#include "hw/hw.h"
>  #include "hw/irq.h"
>  #include "migration/vmstate.h"
>  #include "qemu/module.h"
> @@ -173,8 +171,7 @@ static const VMStateDescription vmstate_max7310 = {
>  static void max7310_gpio_set(void *opaque, int line, int level)
>  {
>      MAX7310State *s = (MAX7310State *) opaque;
> -    if (line >= ARRAY_SIZE(s->handler) || line  < 0)
> -        hw_error("bad GPIO line");
> +    assert(line >= 0 && line < ARRAY_SIZE(s->handler));
>  
>      if (level)
>          s->level |= s->direction & (1 << line);
> 

Applied to my trivial-patches branch.

Thanks,
Laurent

Re: [PATCH v2 2/2] hw/ppc/ppc4xx_pci: Replace magic value by the PCI_NUM_PINS definition

[Laurent Vivier]

Le 10/09/2020 à 09:23, Philippe Mathieu-Daudé a écrit :
> Replace the magic '4' value by the PCI_NUM_PINS definition.
> 
> Suggested-by: Cédric Le Goater <clg@kaod.org>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  hw/ppc/ppc4xx_pci.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/ppc4xx_pci.c b/hw/ppc/ppc4xx_pci.c
> index 503ef46b39a..930be78361d 100644
> --- a/hw/ppc/ppc4xx_pci.c
> +++ b/hw/ppc/ppc4xx_pci.c
> @@ -54,7 +54,7 @@ struct PPC4xxPCIState {
>  
>      struct PCIMasterMap pmm[PPC4xx_PCI_NR_PMMS];
>      struct PCITargetMap ptm[PPC4xx_PCI_NR_PTMS];
> -    qemu_irq irq[4];
> +    qemu_irq irq[PCI_NUM_PINS];
>  
>      MemoryRegion container;
>      MemoryRegion iomem;
> 

Applied to my trivial-patches branch.

Thanks,
Laurent