Coverity points out (CID 1421934) that we are leaking the
memory returned by qemu_allocate_irqs(). We can avoid this
leak by switching to using qdev_init_gpio_in(); the base
class finalize will free the irqs that this allocates under
the hood.

Patch created mechanically using spatch with this script
inspired from commit d6ef883d9d7:

  @@
  typedef qemu_irq;
  identifier irqs, handler;
  expression opaque, count, i;
  @@
  -   qemu_irq *irqs;
      ...
  -   irqs = qemu_allocate_irqs(handler, opaque, count);
  +   qdev_init_gpio_in(DEVICE(opaque), handler, count);
      <+...
  -   irqs[i]
  +   qdev_get_gpio_in(DEVICE(opaque), i)
      ...+>
  ?-  g_free(irqs);

Fixes: Coverity CID 1421934 RESOURCE_LEAK
Inspired-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 hw/openrisc/pic_cpu.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
index 36f9350830..4b0c92f842 100644
--- a/hw/openrisc/pic_cpu.c
+++ b/hw/openrisc/pic_cpu.c
@@ -52,10 +52,9 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
 void cpu_openrisc_pic_init(OpenRISCCPU *cpu)
 {
     int i;
-    qemu_irq *qi;
-    qi = qemu_allocate_irqs(openrisc_pic_cpu_handler, cpu, NR_IRQS);
+    qdev_init_gpio_in(DEVICE(cpu), openrisc_pic_cpu_handler, NR_IRQS);
 
     for (i = 0; i < NR_IRQS; i++) {
-        cpu->env.irq[i] = qi[i];
+        cpu->env.irq[i] = qdev_get_gpio_in(DEVICE(cpu), i);
     }
 }
-- 
2.21.3

On Tue, 12 May 2020 at 08:48, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> Coverity points out (CID 1421934) that we are leaking the
> memory returned by qemu_allocate_irqs(). We can avoid this
> leak by switching to using qdev_init_gpio_in(); the base
> class finalize will free the irqs that this allocates under
> the hood.

>  hw/openrisc/pic_cpu.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/hw/openrisc/pic_cpu.c b/hw/openrisc/pic_cpu.c
> index 36f9350830..4b0c92f842 100644
> --- a/hw/openrisc/pic_cpu.c
> +++ b/hw/openrisc/pic_cpu.c
> @@ -52,10 +52,9 @@ static void openrisc_pic_cpu_handler(void *opaque, int irq, int level)
>  void cpu_openrisc_pic_init(OpenRISCCPU *cpu)
>  {
>      int i;
> -    qemu_irq *qi;
> -    qi = qemu_allocate_irqs(openrisc_pic_cpu_handler, cpu, NR_IRQS);
> +    qdev_init_gpio_in(DEVICE(cpu), openrisc_pic_cpu_handler, NR_IRQS);
>
>      for (i = 0; i < NR_IRQS; i++) {
> -        cpu->env.irq[i] = qi[i];
> +        cpu->env.irq[i] = qdev_get_gpio_in(DEVICE(cpu), i);
>      }
>  }

This isn't wrong as such, but it's a bit weird, because it's code
outside of a device adding GPIO lines to that device, and the
handler function openrisc_pic_cpu_handler() is basically doing
nothing but reaching into the internals of the CPU device and
changing it.

Ideally:
 * all this code should be in target/openrisc/cpu.c, the same
   way the arm CPU creates its own inbound IRQs with qdev_init_gpio_in()
 * cpu->env.irq[] should go away, and hw/openrisc/openrisc_sim.c
   should be calling qdev_get_gpio_in() to get each IRQ line
   it needs, rather than directly grabbing cpu->env.irq and then
   indexing into it

But this change is an OK first step and we can build the other
cleanup on top of it, so
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM