>-----Original Message----- >From: Eric Auger <eric.au...@redhat.com> >Sent: Wednesday, September 27, 2023 5:11 PM >Subject: Re: [PATCH v2 07/12] vfio/platform: Use vfio_[attach/detach]_device > >Hi, > >On 9/26/23 13:32, Zhenzhong Duan wrote: >> From: Eric Auger <eric.au...@redhat.com> >> >> Let the vfio-platform device use vfio_attach_device() and >> vfio_detach_device(), hence hiding the details of the used >> IOMMU backend. >> >> Drop the trace event for vfio-platform as we have similar >> one in vfio_attach_device. >> >> Signed-off-by: Eric Auger <eric.au...@redhat.com> >> Signed-off-by: Yi Liu <yi.l....@intel.com> >> Signed-off-by: Zhenzhong Duan <zhenzhong.d...@intel.com> >> --- >> hw/vfio/platform.c | 43 +++---------------------------------------- >> hw/vfio/trace-events | 1 - >> 2 files changed, 3 insertions(+), 41 deletions(-) >> >> diff --git a/hw/vfio/platform.c b/hw/vfio/platform.c >> index 5af73f9287..8e3d4ac458 100644 >> --- a/hw/vfio/platform.c >> +++ b/hw/vfio/platform.c >> @@ -529,12 +529,7 @@ static VFIODeviceOps vfio_platform_ops = { >> */ >> static int vfio_base_device_init(VFIODevice *vbasedev, Error **errp) >> { >> - VFIOGroup *group; >> - VFIODevice *vbasedev_iter; >> - char *tmp, group_path[PATH_MAX], *group_name; >> - ssize_t len; >> struct stat st; >> - int groupid; >> int ret; >> >> /* @sysfsdev takes precedence over @host */ >> @@ -557,47 +552,15 @@ static int vfio_base_device_init(VFIODevice >*vbasedev, Error **errp) >> return -errno; >> } >> >> - tmp = g_strdup_printf("%s/iommu_group", vbasedev->sysfsdev); >> - len = readlink(tmp, group_path, sizeof(group_path)); >> - g_free(tmp); >> - >> - if (len < 0 || len >= sizeof(group_path)) { >> - ret = len < 0 ? -errno : -ENAMETOOLONG; >> - error_setg_errno(errp, -ret, "no iommu_group found"); >> - return ret; >> - } >> - >> - group_path[len] = 0; >> - >> - group_name = basename(group_path); >> - if (sscanf(group_name, "%d", &groupid) != 1) { >> - error_setg_errno(errp, errno, "failed to read %s", group_path); >> - return -errno; >> - } >> - >Here also on error path we are leaking both vbasedev->name and >vbasedev->sysfsdev. This is independent on this patch >care must be taken because vdev->vbasedev.name is used in the caller >(vfio_platform_realize) to output the error msg >deallocation could happen there?
Both are device property, I think they are freed by QOM subsystem. Please fix me if I'm wrong. static Property vfio_platform_dev_properties[] = { DEFINE_PROP_STRING("host", VFIOPlatformDevice, vbasedev.name), DEFINE_PROP_STRING("sysfsdev", VFIOPlatformDevice, vbasedev.sysfsdev), Thanks Zhenzhong