[PATCH for-8.1] xen: Don't pass MemoryListener around by value

[PATCH for-8.1] xen: Don't pass MemoryListener around by value

[Peter Maydell]

Coverity points out (CID 1513106, 1513107) that MemoryListener is a
192 byte struct which we are passing around by value.  Switch to
passing a const pointer into xen_register_ioreq() and then to
xen_do_ioreq_register().  We can also make the file-scope
MemoryListener variables const, since nothing changes them.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
Disclaimer: I have not tested this beyond any testing you
get from 'make check' and 'make check-avocado', which is likely
not much.
---
 include/hw/xen/xen-hvm-common.h | 2 +-
 hw/arm/xen_arm.c                | 4 ++--
 hw/i386/xen/xen-hvm.c           | 4 ++--
 hw/xen/xen-hvm-common.c         | 8 ++++----
 4 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/hw/xen/xen-hvm-common.h b/include/hw/xen/xen-hvm-common.h
index f9559e2885b..4e9904f1a65 100644
--- a/include/hw/xen/xen-hvm-common.h
+++ b/include/hw/xen/xen-hvm-common.h
@@ -93,7 +93,7 @@ void xen_device_unrealize(DeviceListener *listener, DeviceState *dev);
 
 void xen_hvm_change_state_handler(void *opaque, bool running, RunState rstate);
 void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
-                        MemoryListener xen_memory_listener);
+                        const MemoryListener *xen_memory_listener);
 
 void cpu_ioreq_pio(ioreq_t *req);
 #endif /* HW_XEN_HVM_COMMON_H */
diff --git a/hw/arm/xen_arm.c b/hw/arm/xen_arm.c
index 044093fec75..1d3e6d481a2 100644
--- a/hw/arm/xen_arm.c
+++ b/hw/arm/xen_arm.c
@@ -37,7 +37,7 @@
 #define TYPE_XEN_ARM  MACHINE_TYPE_NAME("xenpvh")
 OBJECT_DECLARE_SIMPLE_TYPE(XenArmState, XEN_ARM)
 
-static MemoryListener xen_memory_listener = {
+static const MemoryListener xen_memory_listener = {
     .region_add = xen_region_add,
     .region_del = xen_region_del,
     .log_start = NULL,
@@ -108,7 +108,7 @@ static void xen_arm_init(MachineState *machine)
 
     xam->state =  g_new0(XenIOState, 1);
 
-    xen_register_ioreq(xam->state, machine->smp.cpus, xen_memory_listener);
+    xen_register_ioreq(xam->state, machine->smp.cpus, &xen_memory_listener);
 
 #ifdef CONFIG_TPM
     if (xam->cfg.tpm_base_addr) {
diff --git a/hw/i386/xen/xen-hvm.c b/hw/i386/xen/xen-hvm.c
index 3da5a2b23f7..f42621e6742 100644
--- a/hw/i386/xen/xen-hvm.c
+++ b/hw/i386/xen/xen-hvm.c
@@ -458,7 +458,7 @@ static void xen_log_global_stop(MemoryListener *listener)
     xen_in_migration = false;
 }
 
-static MemoryListener xen_memory_listener = {
+static const MemoryListener xen_memory_listener = {
     .name = "xen-memory",
     .region_add = xen_region_add,
     .region_del = xen_region_del,
@@ -582,7 +582,7 @@ void xen_hvm_init_pc(PCMachineState *pcms, MemoryRegion **ram_memory)
 
     state = g_new0(XenIOState, 1);
 
-    xen_register_ioreq(state, max_cpus, xen_memory_listener);
+    xen_register_ioreq(state, max_cpus, &xen_memory_listener);
 
     QLIST_INIT(&xen_physmap);
     xen_read_physmap(state);
diff --git a/hw/xen/xen-hvm-common.c b/hw/xen/xen-hvm-common.c
index 886c3ee944d..565dc39c8f6 100644
--- a/hw/xen/xen-hvm-common.c
+++ b/hw/xen/xen-hvm-common.c
@@ -765,8 +765,8 @@ void xen_shutdown_fatal_error(const char *fmt, ...)
 }
 
 static void xen_do_ioreq_register(XenIOState *state,
-                                           unsigned int max_cpus,
-                                           MemoryListener xen_memory_listener)
+                                  unsigned int max_cpus,
+                                  const MemoryListener *xen_memory_listener)
 {
     int i, rc;
 
@@ -824,7 +824,7 @@ static void xen_do_ioreq_register(XenIOState *state,
 
     qemu_add_vm_change_state_handler(xen_hvm_change_state_handler, state);
 
-    state->memory_listener = xen_memory_listener;
+    state->memory_listener = *xen_memory_listener;
     memory_listener_register(&state->memory_listener, &address_space_memory);
 
     state->io_listener = xen_io_listener;
@@ -842,7 +842,7 @@ err:
 }
 
 void xen_register_ioreq(XenIOState *state, unsigned int max_cpus,
-                        MemoryListener xen_memory_listener)
+                        const MemoryListener *xen_memory_listener)
 {
     int rc;
 
-- 
2.34.1

Re: [PATCH for-8.1] xen: Don't pass MemoryListener around by value

[Philippe Mathieu-Daudé]

On 18/7/23 12:10, Peter Maydell wrote:
> Coverity points out (CID 1513106, 1513107) that MemoryListener is a
> 192 byte struct which we are passing around by value.  Switch to
> passing a const pointer into xen_register_ioreq() and then to
> xen_do_ioreq_register().  We can also make the file-scope
> MemoryListener variables const, since nothing changes them.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> Disclaimer: I have not tested this beyond any testing you
> get from 'make check' and 'make check-avocado', which is likely
> not much.
> ---
>   include/hw/xen/xen-hvm-common.h | 2 +-
>   hw/arm/xen_arm.c                | 4 ++--
>   hw/i386/xen/xen-hvm.c           | 4 ++--
>   hw/xen/xen-hvm-common.c         | 8 ++++----
>   4 files changed, 9 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH for-8.1] xen: Don't pass MemoryListener around by value

[Anthony PERARD via]

On Tue, Jul 18, 2023 at 11:10:57AM +0100, Peter Maydell wrote:
> Coverity points out (CID 1513106, 1513107) that MemoryListener is a
> 192 byte struct which we are passing around by value.  Switch to
> passing a const pointer into xen_register_ioreq() and then to
> xen_do_ioreq_register().  We can also make the file-scope
> MemoryListener variables const, since nothing changes them.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>

Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD