1. Patchew
  2. QEMU
  3. View series

[PATCH] vfio/container: Replace basename with g_path_get_basename

Cédric Le Goater posted 1 patch 11 months, 1 week ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20231220135302.932039-1-clg@redhat.com
Maintainers: Alex Williamson <alex.williamson@redhat.com>, "Cédric Le Goater" <clg@redhat.com>
hw/vfio/container.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
[PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Cédric Le Goater 11 months, 1 week ago 
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>
---
 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
RE: [PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Duan, Zhenzhong 11 months, 1 week ago 

>-----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
Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Cédric Le Goater 11 months, 1 week ago 
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;
Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Zhao Liu 11 months, 1 week ago 
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
> 
>
Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Cédric Le Goater 11 months, 1 week ago 
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
>>
>>
>
Re: [PATCH] vfio/container: Replace basename with g_path_get_basename
Posted by Eric Auger 11 months, 1 week ago 
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;

Patchew 2.1-devel-b67e4c2

© 2016 - 2024 Red Hat, Inc.