diff mbox series

[v2] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct

Message ID 20200326204919.22006-1-peter.maydell@linaro.org (mailing list archive)
State New, archived
Headers show
Series [v2] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct | expand

Commit Message

Peter Maydell March 26, 2020, 8:49 p.m. UTC
Coverity complains that the collie_init() function leaks the memory
allocated in sa1110_init().  This is true but not significant since
the function is called only once on machine init and the memory must
remain in existence until QEMU exits anyway.

Still, we can avoid the technical memory leak by keeping the pointer
to the StrongARMState inside the machine state struct.  Switch from
the simple DEFINE_MACHINE() style to defining a subclass of
TYPE_MACHINE which extends the MachineState struct, and keep the
pointer there.

Fixes: CID 1421921
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
v1->v2: folded in the uncommitted change that fixes the
arm_load_kernel() first argument.

 hw/arm/collie.c | 33 ++++++++++++++++++++++++++++-----
 1 file changed, 28 insertions(+), 5 deletions(-)

Comments

Richard Henderson March 26, 2020, 9:52 p.m. UTC | #1
On 3/26/20 1:49 PM, Peter Maydell wrote:
> Coverity complains that the collie_init() function leaks the memory
> allocated in sa1110_init().  This is true but not significant since
> the function is called only once on machine init and the memory must
> remain in existence until QEMU exits anyway.
> 
> Still, we can avoid the technical memory leak by keeping the pointer
> to the StrongARMState inside the machine state struct.  Switch from
> the simple DEFINE_MACHINE() style to defining a subclass of
> TYPE_MACHINE which extends the MachineState struct, and keep the
> pointer there.
> 
> Fixes: CID 1421921
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v1->v2: folded in the uncommitted change that fixes the
> arm_load_kernel() first argument.

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

r~
Philippe Mathieu-Daudé March 27, 2020, 10:46 a.m. UTC | #2
On 3/26/20 9:49 PM, Peter Maydell wrote:
> Coverity complains that the collie_init() function leaks the memory
> allocated in sa1110_init().  This is true but not significant since
> the function is called only once on machine init and the memory must
> remain in existence until QEMU exits anyway.
> 
> Still, we can avoid the technical memory leak by keeping the pointer
> to the StrongARMState inside the machine state struct.  Switch from
> the simple DEFINE_MACHINE() style to defining a subclass of
> TYPE_MACHINE which extends the MachineState struct, and keep the
> pointer there.
> 
> Fixes: CID 1421921
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> v1->v2: folded in the uncommitted change that fixes the
> arm_load_kernel() first argument.
> 
>   hw/arm/collie.c | 33 ++++++++++++++++++++++++++++-----
>   1 file changed, 28 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/arm/collie.c b/hw/arm/collie.c
> index 4992084a3f6..4b35ef4bed6 100644
> --- a/hw/arm/collie.c
> +++ b/hw/arm/collie.c
> @@ -19,6 +19,16 @@
>   #include "exec/address-spaces.h"
>   #include "cpu.h"
>   
> +typedef struct {
> +    MachineState parent;
> +
> +    StrongARMState *sa1110;
> +} CollieMachineState;
> +
> +#define TYPE_COLLIE_MACHINE MACHINE_TYPE_NAME("collie")
> +#define COLLIE_MACHINE(obj) \
> +    OBJECT_CHECK(CollieMachineState, obj, TYPE_COLLIE_MACHINE)
> +
>   static struct arm_boot_info collie_binfo = {
>       .loader_start = SA_SDCS0,
>       .ram_size = 0x20000000,
> @@ -26,9 +36,9 @@ static struct arm_boot_info collie_binfo = {
>   
>   static void collie_init(MachineState *machine)
>   {
> -    StrongARMState *s;
>       DriveInfo *dinfo;
>       MachineClass *mc = MACHINE_GET_CLASS(machine);
> +    CollieMachineState *cms = COLLIE_MACHINE(machine);
>   
>       if (machine->ram_size != mc->default_ram_size) {
>           char *sz = size_to_str(mc->default_ram_size);
> @@ -37,7 +47,7 @@ static void collie_init(MachineState *machine)
>           exit(EXIT_FAILURE);
>       }
>   
> -    s = sa1110_init(machine->cpu_type);
> +    cms->sa1110 = sa1110_init(machine->cpu_type);
>   
>       memory_region_add_subregion(get_system_memory(), SA_SDCS0, machine->ram);
>   
> @@ -54,11 +64,13 @@ static void collie_init(MachineState *machine)
>       sysbus_create_simple("scoop", 0x40800000, NULL);
>   
>       collie_binfo.board_id = 0x208;
> -    arm_load_kernel(s->cpu, machine, &collie_binfo);
> +    arm_load_kernel(cms->sa1110->cpu, machine, &collie_binfo);
>   }
>   
> -static void collie_machine_init(MachineClass *mc)
> +static void collie_machine_class_init(ObjectClass *oc, void *data)
>   {
> +    MachineClass *mc = MACHINE_CLASS(oc);
> +
>       mc->desc = "Sharp SL-5500 (Collie) PDA (SA-1110)";
>       mc->init = collie_init;
>       mc->ignore_memory_transaction_failures = true;
> @@ -67,4 +79,15 @@ static void collie_machine_init(MachineClass *mc)
>       mc->default_ram_id = "strongarm.sdram";
>   }
>   
> -DEFINE_MACHINE("collie", collie_machine_init)
> +static const TypeInfo collie_machine_typeinfo = {
> +    .name = TYPE_COLLIE_MACHINE,
> +    .parent = TYPE_MACHINE,
> +    .class_init = collie_machine_class_init,
> +    .instance_size = sizeof(CollieMachineState),
> +};
> +
> +static void collie_machine_register_types(void)
> +{
> +    type_register_static(&collie_machine_typeinfo);
> +}
> +type_init(collie_machine_register_types);
> 

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

Patch

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 4992084a3f6..4b35ef4bed6 100644
--- a/hw/arm/collie.c
+++ b/hw/arm/collie.c
@@ -19,6 +19,16 @@ 
 #include "exec/address-spaces.h"
 #include "cpu.h"
 
+typedef struct {
+    MachineState parent;
+
+    StrongARMState *sa1110;
+} CollieMachineState;
+
+#define TYPE_COLLIE_MACHINE MACHINE_TYPE_NAME("collie")
+#define COLLIE_MACHINE(obj) \
+    OBJECT_CHECK(CollieMachineState, obj, TYPE_COLLIE_MACHINE)
+
 static struct arm_boot_info collie_binfo = {
     .loader_start = SA_SDCS0,
     .ram_size = 0x20000000,
@@ -26,9 +36,9 @@  static struct arm_boot_info collie_binfo = {
 
 static void collie_init(MachineState *machine)
 {
-    StrongARMState *s;
     DriveInfo *dinfo;
     MachineClass *mc = MACHINE_GET_CLASS(machine);
+    CollieMachineState *cms = COLLIE_MACHINE(machine);
 
     if (machine->ram_size != mc->default_ram_size) {
         char *sz = size_to_str(mc->default_ram_size);
@@ -37,7 +47,7 @@  static void collie_init(MachineState *machine)
         exit(EXIT_FAILURE);
     }
 
-    s = sa1110_init(machine->cpu_type);
+    cms->sa1110 = sa1110_init(machine->cpu_type);
 
     memory_region_add_subregion(get_system_memory(), SA_SDCS0, machine->ram);
 
@@ -54,11 +64,13 @@  static void collie_init(MachineState *machine)
     sysbus_create_simple("scoop", 0x40800000, NULL);
 
     collie_binfo.board_id = 0x208;
-    arm_load_kernel(s->cpu, machine, &collie_binfo);
+    arm_load_kernel(cms->sa1110->cpu, machine, &collie_binfo);
 }
 
-static void collie_machine_init(MachineClass *mc)
+static void collie_machine_class_init(ObjectClass *oc, void *data)
 {
+    MachineClass *mc = MACHINE_CLASS(oc);
+
     mc->desc = "Sharp SL-5500 (Collie) PDA (SA-1110)";
     mc->init = collie_init;
     mc->ignore_memory_transaction_failures = true;
@@ -67,4 +79,15 @@  static void collie_machine_init(MachineClass *mc)
     mc->default_ram_id = "strongarm.sdram";
 }
 
-DEFINE_MACHINE("collie", collie_machine_init)
+static const TypeInfo collie_machine_typeinfo = {
+    .name = TYPE_COLLIE_MACHINE,
+    .parent = TYPE_MACHINE,
+    .class_init = collie_machine_class_init,
+    .instance_size = sizeof(CollieMachineState),
+};
+
+static void collie_machine_register_types(void)
+{
+    type_register_static(&collie_machine_typeinfo);
+}
+type_init(collie_machine_register_types);