>-----Original Message-----
>From: Eric Auger <[email protected]>
>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 <[email protected]>
>>
>> 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 <[email protected]>
>> Signed-off-by: Yi Liu <[email protected]>
>> Signed-off-by: Zhenzhong Duan <[email protected]>
>> ---
>> 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
