* Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
2023-12-20 13:53 [PATCH] vfio/container: Replace basename with g_path_get_basename Cédric Le Goater
@ 2023-12-20 13:59 ` Eric Auger
2023-12-20 15:09 ` Zhao Liu
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: Eric Auger @ 2023-12-20 13:59 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel
Cc: Zhenzhong Duan, Alex Williamson, Khem Raj
Hi Cédric,
On 12/20/23 14:53, Cédric Le Goater wrote:
> g_path_get_basename() is a portable utility function that has the
> advantage of not modifing the string argument. It also fixes a compile
> breakage with the Musl C library reported in [1].
>
> [1] https://lore.kernel.org/all/20231212010228.2701544-1-raj.khem@gmail.com/
>
> Reported-by: Khem Raj <raj.khem@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Eric
> ---
> hw/vfio/container.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f632502e07ffd4dc2ec76cb5 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice *vbasedev)
>
> static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
> {
> - char *tmp, group_path[PATH_MAX], *group_name;
> + char *tmp, group_path[PATH_MAX];
> + g_autofree char *group_name = NULL;
> int ret, groupid;
> ssize_t len;
>
> @@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>
> group_path[len] = 0;
>
> - group_name = basename(group_path);
> + group_name = g_path_get_basename(group_path);
> if (sscanf(group_name, "%d", &groupid) != 1) {
> error_setg_errno(errp, errno, "failed to read %s", group_path);
> return -errno;
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
2023-12-20 13:53 [PATCH] vfio/container: Replace basename with g_path_get_basename Cédric Le Goater
2023-12-20 13:59 ` Eric Auger
@ 2023-12-20 15:09 ` Zhao Liu
2023-12-20 17:05 ` Cédric Le Goater
2023-12-20 17:01 ` Cédric Le Goater
2023-12-21 2:44 ` Duan, Zhenzhong
3 siblings, 1 reply; 6+ messages in thread
From: Zhao Liu @ 2023-12-20 15:09 UTC (permalink / raw)
To: Cédric Le Goater
Cc: qemu-devel, Eric Auger, Zhenzhong Duan, Alex Williamson, Khem Raj
Hi Cédric,
On Wed, Dec 20, 2023 at 02:53:02PM +0100, Cédric Le Goater wrote:
> Date: Wed, 20 Dec 2023 14:53:02 +0100
> From: Cédric Le Goater <clg@redhat.com>
> Subject: [PATCH] vfio/container: Replace basename with g_path_get_basename
> X-Mailer: git-send-email 2.43.0
>
> g_path_get_basename() is a portable utility function that has the
> advantage of not modifing the string argument. It also fixes a compile
> breakage with the Musl C library reported in [1].
>
> [1] https://lore.kernel.org/all/20231212010228.2701544-1-raj.khem@gmail.com/
>
> Reported-by: Khem Raj <raj.khem@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
> ---
Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
Just one additional question, I understand that QEMU should replace all
basename() with g_path_get_basename(), right?
I find hw/s390x/s390-ccw.c also uses basename(). Maybe I can clean it up
to avoid potentially similar issue.
Thanks,
Zhao
> hw/vfio/container.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f632502e07ffd4dc2ec76cb5 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice *vbasedev)
>
> static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
> {
> - char *tmp, group_path[PATH_MAX], *group_name;
> + char *tmp, group_path[PATH_MAX];
> + g_autofree char *group_name = NULL;
> int ret, groupid;
> ssize_t len;
>
> @@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>
> group_path[len] = 0;
>
> - group_name = basename(group_path);
> + group_name = g_path_get_basename(group_path);
> if (sscanf(group_name, "%d", &groupid) != 1) {
> error_setg_errno(errp, errno, "failed to read %s", group_path);
> return -errno;
> --
> 2.43.0
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
2023-12-20 15:09 ` Zhao Liu
@ 2023-12-20 17:05 ` Cédric Le Goater
0 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2023-12-20 17:05 UTC (permalink / raw)
To: Zhao Liu
Cc: qemu-devel, Eric Auger, Zhenzhong Duan, Alex Williamson, Khem Raj
Hello,
On 12/20/23 16:09, Zhao Liu wrote:
> Hi Cédric,
>
> On Wed, Dec 20, 2023 at 02:53:02PM +0100, Cédric Le Goater wrote:
>> Date: Wed, 20 Dec 2023 14:53:02 +0100
>> From: Cédric Le Goater <clg@redhat.com>
>> Subject: [PATCH] vfio/container: Replace basename with g_path_get_basename
>> X-Mailer: git-send-email 2.43.0
>>
>> g_path_get_basename() is a portable utility function that has the
>> advantage of not modifing the string argument. It also fixes a compile
>> breakage with the Musl C library reported in [1].
>>
>> [1] https://lore.kernel.org/all/20231212010228.2701544-1-raj.khem@gmail.com/
>>
>> Reported-by: Khem Raj <raj.khem@gmail.com>
>> Signed-off-by: Cédric Le Goater <clg@redhat.com>
>> ---
>
> Reviewed-by: Zhao Liu <zhao1.liu@intel.com>
>
> Just one additional question, I understand that QEMU should replace all
> basename() with g_path_get_basename(), right?
>
> I find hw/s390x/s390-ccw.c also uses basename(). Maybe I can clean it up
> to avoid potentially similar issue.
I guess so and I wonder why 3e015d815b3f didn't do the change.
Anyhow, please cc me, I can give it a try.
Thanks,
C.
>
> Thanks,
> Zhao
>
>> hw/vfio/container.c | 5 +++--
>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>
>> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>> index 688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f632502e07ffd4dc2ec76cb5 100644
>> --- a/hw/vfio/container.c
>> +++ b/hw/vfio/container.c
>> @@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice *vbasedev)
>>
>> static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>> {
>> - char *tmp, group_path[PATH_MAX], *group_name;
>> + char *tmp, group_path[PATH_MAX];
>> + g_autofree char *group_name = NULL;
>> int ret, groupid;
>> ssize_t len;
>>
>> @@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>>
>> group_path[len] = 0;
>>
>> - group_name = basename(group_path);
>> + group_name = g_path_get_basename(group_path);
>> if (sscanf(group_name, "%d", &groupid) != 1) {
>> error_setg_errno(errp, errno, "failed to read %s", group_path);
>> return -errno;
>> --
>> 2.43.0
>>
>>
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
2023-12-20 13:53 [PATCH] vfio/container: Replace basename with g_path_get_basename Cédric Le Goater
2023-12-20 13:59 ` Eric Auger
2023-12-20 15:09 ` Zhao Liu
@ 2023-12-20 17:01 ` Cédric Le Goater
2023-12-21 2:44 ` Duan, Zhenzhong
3 siblings, 0 replies; 6+ messages in thread
From: Cédric Le Goater @ 2023-12-20 17:01 UTC (permalink / raw)
To: qemu-devel; +Cc: Eric Auger, Zhenzhong Duan, Alex Williamson, Khem Raj
On 12/20/23 14:53, Cédric Le Goater wrote:
> g_path_get_basename() is a portable utility function that has the
> advantage of not modifing the string argument. It also fixes a compile
> breakage with the Musl C library reported in [1].
>
> [1] https://lore.kernel.org/all/20231212010228.2701544-1-raj.khem@gmail.com/
>
> Reported-by: Khem Raj <raj.khem@gmail.com>
> Signed-off-by: Cédric Le Goater <clg@redhat.com>
Applied to vfio-next.
Thanks,
C.
> ---
> hw/vfio/container.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/hw/vfio/container.c b/hw/vfio/container.c
> index 688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f632502e07ffd4dc2ec76cb5 100644
> --- a/hw/vfio/container.c
> +++ b/hw/vfio/container.c
> @@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice *vbasedev)
>
> static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
> {
> - char *tmp, group_path[PATH_MAX], *group_name;
> + char *tmp, group_path[PATH_MAX];
> + g_autofree char *group_name = NULL;
> int ret, groupid;
> ssize_t len;
>
> @@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
>
> group_path[len] = 0;
>
> - group_name = basename(group_path);
> + group_name = g_path_get_basename(group_path);
> if (sscanf(group_name, "%d", &groupid) != 1) {
> error_setg_errno(errp, errno, "failed to read %s", group_path);
> return -errno;
^ permalink raw reply [flat|nested] 6+ messages in thread* RE: [PATCH] vfio/container: Replace basename with g_path_get_basename
2023-12-20 13:53 [PATCH] vfio/container: Replace basename with g_path_get_basename Cédric Le Goater
` (2 preceding siblings ...)
2023-12-20 17:01 ` Cédric Le Goater
@ 2023-12-21 2:44 ` Duan, Zhenzhong
3 siblings, 0 replies; 6+ messages in thread
From: Duan, Zhenzhong @ 2023-12-21 2:44 UTC (permalink / raw)
To: Cédric Le Goater, qemu-devel@nongnu.org
Cc: Eric Auger, Alex Williamson, Khem Raj
>-----Original Message-----
>From: Cédric Le Goater <clg@redhat.com>
>Subject: [PATCH] vfio/container: Replace basename with
>g_path_get_basename
>
>g_path_get_basename() is a portable utility function that has the
>advantage of not modifing the string argument. It also fixes a compile
>breakage with the Musl C library reported in [1].
>
>[1] https://lore.kernel.org/all/20231212010228.2701544-1-
>raj.khem@gmail.com/
>
>Reported-by: Khem Raj <raj.khem@gmail.com>
>Signed-off-by: Cédric Le Goater <clg@redhat.com>
Reviewed-by: Zhenzhong Duan <zhenzhong.duan@intel.com>
Thanks
Zhenzhong
>---
> hw/vfio/container.c | 5 +++--
> 1 file changed, 3 insertions(+), 2 deletions(-)
>
>diff --git a/hw/vfio/container.c b/hw/vfio/container.c
>index
>688cf23bab88f85246378bc5a7da3c51ea6b79d9..8d334f52f2438d05f63250
>2e07ffd4dc2ec76cb5 100644
>--- a/hw/vfio/container.c
>+++ b/hw/vfio/container.c
>@@ -869,7 +869,8 @@ static void vfio_put_base_device(VFIODevice
>*vbasedev)
>
> static int vfio_device_groupid(VFIODevice *vbasedev, Error **errp)
> {
>- char *tmp, group_path[PATH_MAX], *group_name;
>+ char *tmp, group_path[PATH_MAX];
>+ g_autofree char *group_name = NULL;
> int ret, groupid;
> ssize_t len;
>
>@@ -885,7 +886,7 @@ static int vfio_device_groupid(VFIODevice
>*vbasedev, Error **errp)
>
> group_path[len] = 0;
>
>- group_name = basename(group_path);
>+ group_name = g_path_get_basename(group_path);
> if (sscanf(group_name, "%d", &groupid) != 1) {
> error_setg_errno(errp, errno, "failed to read %s", group_path);
> return -errno;
>--
>2.43.0
^ permalink raw reply [flat|nested] 6+ messages in thread