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

Peter Maydell posted 1 patch 4 years, 1 month ago
Test docker-mingw@fedora failed
Test docker-quick@centos7 failed
Test checkpatch passed
Test FreeBSD passed
Test asan passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20200326203414.18140-1-peter.maydell@linaro.org
Maintainers: Peter Maydell <peter.maydell@linaro.org>
There is a newer version of this series
hw/arm/collie.c | 31 +++++++++++++++++++++++++++----
1 file changed, 27 insertions(+), 4 deletions(-)
[PATCH] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct
Posted by Peter Maydell 4 years, 1 month ago
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>
---
 hw/arm/collie.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)

diff --git a/hw/arm/collie.c b/hw/arm/collie.c
index 4992084a3f6..8ca0df96b47 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);
 
@@ -57,8 +67,10 @@ static void collie_init(MachineState *machine)
     arm_load_kernel(s->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);
-- 
2.20.1


Re: [PATCH] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct
Posted by Peter Maydell 4 years, 1 month ago
On Thu, 26 Mar 2020 at 20:34, Peter Maydell <peter.maydell@linaro.org> 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>

Rats, sent this out and immediately noticed I'd forgotten
to commit a change. v2 coming up...

-- PMM

Re: [PATCH] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct
Posted by no-reply@patchew.org 4 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20200326203414.18140-1-peter.maydell@linaro.org/



Hi,

This series failed the docker-quick@centos7 build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#!/bin/bash
make docker-image-centos7 V=1 NETWORK=1
time make docker-test-quick@centos7 SHOW_ENV=1 J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      x86_64-softmmu/hw/i386/kvm/apic.o
  CC      x86_64-softmmu/hw/i386/kvm/ioapic.o
/tmp/qemu-test/src/hw/arm/collie.c: In function 'collie_init':
/tmp/qemu-test/src/hw/arm/collie.c:67:21: error: 's' undeclared (first use in this function)
     arm_load_kernel(s->cpu, machine, &collie_binfo);
                     ^
/tmp/qemu-test/src/hw/arm/collie.c:67:21: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [hw/arm/collie.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/hw/i386/kvm/i8254.o
  CC      x86_64-softmmu/hw/i386/kvm/i8259.o
---
  CC      x86_64-softmmu/qapi/qapi-types.o
  CC      x86_64-softmmu/qapi/qapi-visit-machine-target.o
  CC      x86_64-softmmu/qapi/qapi-visit-misc-target.o
make: *** [aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  CC      x86_64-softmmu/qapi/qapi-visit.o
  CC      x86_64-softmmu/qapi/qapi-events-machine-target.o
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=8f0344cce85a42aba1583437563b4300', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=1', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-usmfb9pp/src/docker-src.2020-03-26-17.17.10.30983:/var/tmp/qemu:z,ro', 'qemu:centos7', '/var/tmp/qemu/run', 'test-quick']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=8f0344cce85a42aba1583437563b4300
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-usmfb9pp/src'
make: *** [docker-run-test-quick@centos7] Error 2

real    2m33.540s
user    0m8.690s


The full log is available at
http://patchew.org/logs/20200326203414.18140-1-peter.maydell@linaro.org/testing.docker-quick@centos7/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com
Re: [PATCH] hw/arm/collie: Put StrongARMState* into a CollieMachineState struct
Posted by no-reply@patchew.org 4 years, 1 month ago
Patchew URL: https://patchew.org/QEMU/20200326203414.18140-1-peter.maydell@linaro.org/



Hi,

This series failed the docker-mingw@fedora build test. Please find the testing commands and
their output below. If you have Docker installed, you can probably reproduce it
locally.

=== TEST SCRIPT BEGIN ===
#! /bin/bash
export ARCH=x86_64
make docker-image-fedora V=1 NETWORK=1
time make docker-test-mingw@fedora J=14 NETWORK=1
=== TEST SCRIPT END ===

  CC      aarch64-softmmu/target/arm/translate-a64.o
  CC      aarch64-softmmu/target/arm/helper-a64.o
/tmp/qemu-test/src/hw/arm/collie.c: In function 'collie_init':
/tmp/qemu-test/src/hw/arm/collie.c:67:21: error: 's' undeclared (first use in this function)
     arm_load_kernel(s->cpu, machine, &collie_binfo);
                     ^
/tmp/qemu-test/src/hw/arm/collie.c:67:21: note: each undeclared identifier is reported only once for each function it appears in
make[1]: *** [/tmp/qemu-test/src/rules.mak:69: hw/arm/collie.o] Error 1
make[1]: *** Waiting for unfinished jobs....
  GEN     aarch64-softmmu/target/arm/decode-sve.inc.c
make: *** [Makefile:527: aarch64-softmmu/all] Error 2
make: *** Waiting for unfinished jobs....
  GEN     x86_64-softmmu/qemu-system-x86_64.exe
Traceback (most recent call last):
---
    raise CalledProcessError(retcode, cmd)
subprocess.CalledProcessError: Command '['sudo', '-n', 'docker', 'run', '--label', 'com.qemu.instance.uuid=cdb914bf66524d7ea3f103d22d1219a7', '-u', '1001', '--security-opt', 'seccomp=unconfined', '--rm', '-e', 'TARGET_LIST=', '-e', 'EXTRA_CONFIGURE_OPTS=', '-e', 'V=', '-e', 'J=14', '-e', 'DEBUG=', '-e', 'SHOW_ENV=', '-e', 'CCACHE_DIR=/var/tmp/ccache', '-v', '/home/patchew/.cache/qemu-docker-ccache:/var/tmp/ccache:z', '-v', '/var/tmp/patchew-tester-tmp-ugzz1zvo/src/docker-src.2020-03-26-17.20.21.7751:/var/tmp/qemu:z,ro', 'qemu:fedora', '/var/tmp/qemu/run', 'test-mingw']' returned non-zero exit status 2.
filter=--filter=label=com.qemu.instance.uuid=cdb914bf66524d7ea3f103d22d1219a7
make[1]: *** [docker-run] Error 1
make[1]: Leaving directory `/var/tmp/patchew-tester-tmp-ugzz1zvo/src'
make: *** [docker-run-test-mingw@fedora] Error 2

real    2m14.621s
user    0m8.313s


The full log is available at
http://patchew.org/logs/20200326203414.18140-1-peter.maydell@linaro.org/testing.docker-mingw@fedora/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com