diff mbox series

[for-3.1] hw/ppc/mac_newworld: Free openpic_irqs array after use

Message ID 20181101161758.12300-1-peter.maydell@linaro.org
State Superseded
Headers show
Series [for-3.1] hw/ppc/mac_newworld: Free openpic_irqs array after use | expand

Commit Message

Peter Maydell Nov. 1, 2018, 4:17 p.m. UTC
In ppc_core99_init(), we allocate an openpic_irqs array, which
we then use to collect up the various qemu_irqs which we're
going to connect to the interrupt controller. Once we've
called sysbus_connect_irq() to connect them all up, the
array is no longer required, but we forgot to free it.

Since board init is only run once at startup, the memory
leak is not a significant one.

Spotted by Coverity: CID 1192916.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

---
 hw/ppc/mac_newworld.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.19.1

Comments

Philippe Mathieu-Daudé Nov. 1, 2018, 5:37 p.m. UTC | #1
On 1/11/18 17:17, Peter Maydell wrote:
> In ppc_core99_init(), we allocate an openpic_irqs array, which

> we then use to collect up the various qemu_irqs which we're

> going to connect to the interrupt controller. Once we've

> called sysbus_connect_irq() to connect them all up, the

> array is no longer required, but we forgot to free it.

> 

> Since board init is only run once at startup, the memory

> leak is not a significant one.

> 

> Spotted by Coverity: CID 1192916.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> ---

>   hw/ppc/mac_newworld.c | 1 +

>   1 file changed, 1 insertion(+)

> 

> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c

> index a630cb81cd8..14273a123e5 100644

> --- a/hw/ppc/mac_newworld.c

> +++ b/hw/ppc/mac_newworld.c

> @@ -303,6 +303,7 @@ static void ppc_core99_init(MachineState *machine)

>               sysbus_connect_irq(s, k++, openpic_irqs[i][j]);

>           }

>       }

> +    g_free(openpic_irqs);

>   

>       if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {

>           /* 970 gets a U3 bus */

>
Mark Cave-Ayland Nov. 2, 2018, 11:14 a.m. UTC | #2
On 01/11/2018 16:17, Peter Maydell wrote:

> In ppc_core99_init(), we allocate an openpic_irqs array, which

> we then use to collect up the various qemu_irqs which we're

> going to connect to the interrupt controller. Once we've

> called sysbus_connect_irq() to connect them all up, the

> array is no longer required, but we forgot to free it.

> 

> Since board init is only run once at startup, the memory

> leak is not a significant one.

> 

> Spotted by Coverity: CID 1192916.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

> ---

>  hw/ppc/mac_newworld.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c

> index a630cb81cd8..14273a123e5 100644

> --- a/hw/ppc/mac_newworld.c

> +++ b/hw/ppc/mac_newworld.c

> @@ -303,6 +303,7 @@ static void ppc_core99_init(MachineState *machine)

>              sysbus_connect_irq(s, k++, openpic_irqs[i][j]);

>          }

>      }

> +    g_free(openpic_irqs);

>  

>      if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {

>          /* 970 gets a U3 bus */

> 


Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>


I did notice the generation of this 2D array for the OpenPIC controller whilst
converting the Mac machines over to qdev, but wasn't exactly sure what to do here so
I left it.


ATB,

Mark.
Peter Maydell Nov. 2, 2018, 11:19 a.m. UTC | #3
On 2 November 2018 at 11:14, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
> On 01/11/2018 16:17, Peter Maydell wrote:

>

>> In ppc_core99_init(), we allocate an openpic_irqs array, which

>> we then use to collect up the various qemu_irqs which we're

>> going to connect to the interrupt controller. Once we've

>> called sysbus_connect_irq() to connect them all up, the

>> array is no longer required, but we forgot to free it.

>>

>> Since board init is only run once at startup, the memory

>> leak is not a significant one.

>>

>> Spotted by Coverity: CID 1192916.

>>

>> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

>> ---

>>  hw/ppc/mac_newworld.c | 1 +

>>  1 file changed, 1 insertion(+)

>>

>> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c

>> index a630cb81cd8..14273a123e5 100644

>> --- a/hw/ppc/mac_newworld.c

>> +++ b/hw/ppc/mac_newworld.c

>> @@ -303,6 +303,7 @@ static void ppc_core99_init(MachineState *machine)

>>              sysbus_connect_irq(s, k++, openpic_irqs[i][j]);

>>          }

>>      }

>> +    g_free(openpic_irqs);

>>

>>      if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {

>>          /* 970 gets a U3 bus */

>>

>

> Reviewed-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>

>

> I did notice the generation of this 2D array for the OpenPIC controller whilst

> converting the Mac machines over to qdev, but wasn't exactly sure what to do here so

> I left it.


In some sense the array isn't really necessary at all -- instead
of "fill in array with things; create PIC; sysbus_connect_irq from array"
you could just do "create PIC; sysbus_connect_irq to things".
But for this patch I opted to just free the memory rather than
attempt more complicated refactoring.

thanks
-- PMM
David Gibson Nov. 3, 2018, 12:59 p.m. UTC | #4
On Thu, Nov 01, 2018 at 04:17:58PM +0000, Peter Maydell wrote:
> In ppc_core99_init(), we allocate an openpic_irqs array, which

> we then use to collect up the various qemu_irqs which we're

> going to connect to the interrupt controller. Once we've

> called sysbus_connect_irq() to connect them all up, the

> array is no longer required, but we forgot to free it.

> 

> Since board init is only run once at startup, the memory

> leak is not a significant one.

> 

> Spotted by Coverity: CID 1192916.

> 

> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>


Applied, thanks.

> ---

>  hw/ppc/mac_newworld.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c

> index a630cb81cd8..14273a123e5 100644

> --- a/hw/ppc/mac_newworld.c

> +++ b/hw/ppc/mac_newworld.c

> @@ -303,6 +303,7 @@ static void ppc_core99_init(MachineState *machine)

>              sysbus_connect_irq(s, k++, openpic_irqs[i][j]);

>          }

>      }

> +    g_free(openpic_irqs);

>  

>      if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {

>          /* 970 gets a U3 bus */


-- 
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
diff mbox series

Patch

diff --git a/hw/ppc/mac_newworld.c b/hw/ppc/mac_newworld.c
index a630cb81cd8..14273a123e5 100644
--- a/hw/ppc/mac_newworld.c
+++ b/hw/ppc/mac_newworld.c
@@ -303,6 +303,7 @@  static void ppc_core99_init(MachineState *machine)
             sysbus_connect_irq(s, k++, openpic_irqs[i][j]);
         }
     }
+    g_free(openpic_irqs);
 
     if (PPC_INPUT(env) == PPC_FLAGS_INPUT_970) {
         /* 970 gets a U3 bus */