Re: [PATCH v2 17/20] vfio: Use g_autofree in all call site of vfio_get_region_info()
On 5/22/24 06:40, Zhenzhong Duan wrote:
There are some exceptions when pointer to vfio_region_info is reused.
In that case, the pointed memory is freed manually.
Suggested-by: Cédric Le Goater
Signed-off-by: Zhenzhong Duan
Reviewed-by: Cédric Le Goater
Thanks,
C.
---
hw/vfio/helpers.c | 7 ++-
hw/vfio/igd.c | 5 ++---
hw/vfio/pci.c | 13 +++--
3 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index 4b079dc383..27ea26aa48 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -343,7 +343,7 @@ static int vfio_setup_region_sparse_mmaps(VFIORegion
*region,
int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
int index, const char *name)
{
-struct vfio_region_info *info;
+g_autofree struct vfio_region_info *info = NULL;
int ret;
ret = vfio_get_region_info(vbasedev, index, );
@@ -376,8 +376,6 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev,
VFIORegion *region,
}
}
-g_free(info);
-
trace_vfio_region_setup(vbasedev->name, index, name,
region->flags, region->fd_offset, region->size);
return 0;
@@ -594,14 +592,13 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev,
uint32_t type,
bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type)
{
-struct vfio_region_info *info = NULL;
+g_autofree struct vfio_region_info *info = NULL;
bool ret = false;
if (!vfio_get_region_info(vbasedev, region, )) {
if (vfio_get_region_info_cap(info, cap_type)) {
ret = true;
}
-g_free(info);
}
return ret;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 402fc5ce1d..1e79202f2b 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -367,8 +367,8 @@ static const MemoryRegionOps vfio_igd_index_quirk = {
void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
{
-struct vfio_region_info *rom = NULL, *opregion = NULL,
-*host = NULL, *lpc = NULL;
+g_autofree struct vfio_region_info *rom = NULL;
+struct vfio_region_info *opregion = NULL, *host = NULL, *lpc = NULL;
VFIOQuirk *quirk;
VFIOIGDQuirk *igd;
PCIDevice *lpc_bridge;
@@ -609,7 +609,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb);
out:
-g_free(rom);
g_free(opregion);
g_free(host);
g_free(lpc);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 35ad9b582f..74a79bdf61 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -879,7 +879,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
{
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
uint64_t size;
off_t off = 0;
ssize_t bytes;
@@ -897,8 +897,6 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
vdev->rom_size = size = reg_info->size;
vdev->rom_offset = reg_info->offset;
-g_free(reg_info);
-
if (!vdev->rom_size) {
vdev->rom_read_failed = true;
error_report("vfio-pci: Cannot read device rom at "
@@ -2668,7 +2666,7 @@ static VFIODeviceOps vfio_pci_ops = {
bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
{
VFIODevice *vbasedev = >vbasedev;
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
int ret;
ret = vfio_get_region_info(vbasedev, VFIO_PCI_VGA_REGION_INDEX, _info);
@@ -2685,7 +2683,6 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
error_setg(errp, "unexpected VGA info, flags 0x%lx, size 0x%lx",
(unsigned long)reg_info->flags,
(unsigned long)reg_info->size);
-g_free(reg_info);
return false;
}
@@ -2694,8 +2691,6 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
vdev->vga->fd_offset = reg_info->offset;
vdev->vga->fd = vdev->vbasedev.fd;
-g_free(reg_info);
-
vdev->vga->region[QEMU_PCI_VGA_MEM].offset = QEMU_PCI_VGA_MEM_BASE;
vdev->vga->region[QEMU_PCI_VGA_MEM].nr = QEMU_PCI_VGA_MEM;
QLIST_INIT(>vga->region[QEMU_PCI_VGA_MEM].quirks);
@@ -2736,7 +2731,7 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
{
VFIODevice *vbasedev = >vbasedev;
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
int i, ret = -1;
@@ -2790,8 +2785,6 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
}
vdev->config_offset = reg_info->offset;
-g_free(reg_info);
-
if (vdev->features &
[PATCH v2 17/20] vfio: Use g_autofree in all call site of vfio_get_region_info()
There are some exceptions when pointer to vfio_region_info is reused.
In that case, the pointed memory is freed manually.
Suggested-by: Cédric Le Goater
Signed-off-by: Zhenzhong Duan
---
hw/vfio/helpers.c | 7 ++-
hw/vfio/igd.c | 5 ++---
hw/vfio/pci.c | 13 +++--
3 files changed, 7 insertions(+), 18 deletions(-)
diff --git a/hw/vfio/helpers.c b/hw/vfio/helpers.c
index 4b079dc383..27ea26aa48 100644
--- a/hw/vfio/helpers.c
+++ b/hw/vfio/helpers.c
@@ -343,7 +343,7 @@ static int vfio_setup_region_sparse_mmaps(VFIORegion
*region,
int vfio_region_setup(Object *obj, VFIODevice *vbasedev, VFIORegion *region,
int index, const char *name)
{
-struct vfio_region_info *info;
+g_autofree struct vfio_region_info *info = NULL;
int ret;
ret = vfio_get_region_info(vbasedev, index, );
@@ -376,8 +376,6 @@ int vfio_region_setup(Object *obj, VFIODevice *vbasedev,
VFIORegion *region,
}
}
-g_free(info);
-
trace_vfio_region_setup(vbasedev->name, index, name,
region->flags, region->fd_offset, region->size);
return 0;
@@ -594,14 +592,13 @@ int vfio_get_dev_region_info(VFIODevice *vbasedev,
uint32_t type,
bool vfio_has_region_cap(VFIODevice *vbasedev, int region, uint16_t cap_type)
{
-struct vfio_region_info *info = NULL;
+g_autofree struct vfio_region_info *info = NULL;
bool ret = false;
if (!vfio_get_region_info(vbasedev, region, )) {
if (vfio_get_region_info_cap(info, cap_type)) {
ret = true;
}
-g_free(info);
}
return ret;
diff --git a/hw/vfio/igd.c b/hw/vfio/igd.c
index 402fc5ce1d..1e79202f2b 100644
--- a/hw/vfio/igd.c
+++ b/hw/vfio/igd.c
@@ -367,8 +367,8 @@ static const MemoryRegionOps vfio_igd_index_quirk = {
void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
{
-struct vfio_region_info *rom = NULL, *opregion = NULL,
-*host = NULL, *lpc = NULL;
+g_autofree struct vfio_region_info *rom = NULL;
+struct vfio_region_info *opregion = NULL, *host = NULL, *lpc = NULL;
VFIOQuirk *quirk;
VFIOIGDQuirk *igd;
PCIDevice *lpc_bridge;
@@ -609,7 +609,6 @@ void vfio_probe_igd_bar4_quirk(VFIOPCIDevice *vdev, int nr)
trace_vfio_pci_igd_bdsm_enabled(vdev->vbasedev.name, ggms_mb + gms_mb);
out:
-g_free(rom);
g_free(opregion);
g_free(host);
g_free(lpc);
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c
index 35ad9b582f..74a79bdf61 100644
--- a/hw/vfio/pci.c
+++ b/hw/vfio/pci.c
@@ -879,7 +879,7 @@ static void vfio_update_msi(VFIOPCIDevice *vdev)
static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
{
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
uint64_t size;
off_t off = 0;
ssize_t bytes;
@@ -897,8 +897,6 @@ static void vfio_pci_load_rom(VFIOPCIDevice *vdev)
vdev->rom_size = size = reg_info->size;
vdev->rom_offset = reg_info->offset;
-g_free(reg_info);
-
if (!vdev->rom_size) {
vdev->rom_read_failed = true;
error_report("vfio-pci: Cannot read device rom at "
@@ -2668,7 +2666,7 @@ static VFIODeviceOps vfio_pci_ops = {
bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
{
VFIODevice *vbasedev = >vbasedev;
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
int ret;
ret = vfio_get_region_info(vbasedev, VFIO_PCI_VGA_REGION_INDEX, _info);
@@ -2685,7 +2683,6 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
error_setg(errp, "unexpected VGA info, flags 0x%lx, size 0x%lx",
(unsigned long)reg_info->flags,
(unsigned long)reg_info->size);
-g_free(reg_info);
return false;
}
@@ -2694,8 +2691,6 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
vdev->vga->fd_offset = reg_info->offset;
vdev->vga->fd = vdev->vbasedev.fd;
-g_free(reg_info);
-
vdev->vga->region[QEMU_PCI_VGA_MEM].offset = QEMU_PCI_VGA_MEM_BASE;
vdev->vga->region[QEMU_PCI_VGA_MEM].nr = QEMU_PCI_VGA_MEM;
QLIST_INIT(>vga->region[QEMU_PCI_VGA_MEM].quirks);
@@ -2736,7 +2731,7 @@ bool vfio_populate_vga(VFIOPCIDevice *vdev, Error **errp)
static bool vfio_populate_device(VFIOPCIDevice *vdev, Error **errp)
{
VFIODevice *vbasedev = >vbasedev;
-struct vfio_region_info *reg_info;
+g_autofree struct vfio_region_info *reg_info = NULL;
struct vfio_irq_info irq_info = { .argsz = sizeof(irq_info) };
int i, ret = -1;
@@ -2790,8 +2785,6 @@ static bool vfio_populate_device(VFIOPCIDevice *vdev,
Error **errp)
}
vdev->config_offset = reg_info->offset;
-g_free(reg_info);
-
if (vdev->features & VFIO_FEATURE_ENABLE_VGA) {
if (!vfio_populate_vga(vdev, errp)) {
error_append_hint(errp, "device does not support "
--
2.34.1
