[Qemu-devel] [PATCH] spapr: drop useless sanity check in spapr_irq_alloc*()

[Qemu-devel] [PATCH] spapr: drop useless sanity check in spapr_irq_alloc*()

[Greg Kurz]

Both spapr_irq_alloc() and spapr_irq_alloc_block() have an errp
parameter, but they don't use it if XICS hasn't been initialized
yet.

This is doubly wrong:

- all callers do pass a non-null Error **, ie, they expect an error
  to be propagated in case of failure

- XICS obviously needs to be initialized before anything starts allocating
  IRQs

So this patch turns the check into an assert.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c |    9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 4d27909fb152..799673319b06 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3707,9 +3707,8 @@ int spapr_irq_alloc(sPAPRMachineState *spapr, int irq_hint, bool lsi,
     ICSState *ics = spapr->ics;
     int irq;
 
-    if (!ics) {
-        return -1;
-    }
+    assert(ics);
+
     if (irq_hint) {
         if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
             error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
@@ -3741,9 +3740,7 @@ int spapr_irq_alloc_block(sPAPRMachineState *spapr, int num, bool lsi,
     ICSState *ics = spapr->ics;
     int i, first = -1;
 
-    if (!ics) {
-        return -1;
-    }
+    assert(ics);
 
     /*
      * MSIMesage::data is used for storing VIRQ so

Re: [Qemu-devel] [PATCH] spapr: drop useless sanity check in spapr_irq_alloc*()

[David Gibson]

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

On Wed, Apr 11, 2018 at 07:46:06PM +0200, Greg Kurz wrote:
> Both spapr_irq_alloc() and spapr_irq_alloc_block() have an errp
> parameter, but they don't use it if XICS hasn't been initialized
> yet.
> 
> This is doubly wrong:
> 
> - all callers do pass a non-null Error **, ie, they expect an error
>   to be propagated in case of failure
> 
> - XICS obviously needs to be initialized before anything starts allocating
>   IRQs
> 
> So this patch turns the check into an assert.
> 
> Signed-off-by: Greg Kurz <groug@kaod.org>

Applied to ppc-for-2.13, thanks.

> ---
>  hw/ppc/spapr.c |    9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 4d27909fb152..799673319b06 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3707,9 +3707,8 @@ int spapr_irq_alloc(sPAPRMachineState *spapr, int irq_hint, bool lsi,
>      ICSState *ics = spapr->ics;
>      int irq;
>  
> -    if (!ics) {
> -        return -1;
> -    }
> +    assert(ics);
> +
>      if (irq_hint) {
>          if (!ICS_IRQ_FREE(ics, irq_hint - ics->offset)) {
>              error_setg(errp, "can't allocate IRQ %d: already in use", irq_hint);
> @@ -3741,9 +3740,7 @@ int spapr_irq_alloc_block(sPAPRMachineState *spapr, int num, bool lsi,
>      ICSState *ics = spapr->ics;
>      int i, first = -1;
>  
> -    if (!ics) {
> -        return -1;
> -    }
> +    assert(ics);
>  
>      /*
>       * MSIMesage::data is used for storing VIRQ so
> 

-- 
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 --]