RE: [PATCH] vfio/container: Replace basename with g_path_get_basename

2023-12-20 Thread Duan, Zhenzhong 


>-Original Message-
>From: Cédric Le Goater 
>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.k...@gmail.com/
>
>Reported-by: Khem Raj 
>Signed-off-by: Cédric Le Goater 

Reviewed-by: Zhenzhong Duan 

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", ) != 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

2023-12-20 Thread Cédric Le Goater 

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 
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.k...@gmail.com/

Reported-by: Khem Raj 
Signed-off-by: Cédric Le Goater 
---


Reviewed-by: Zhao Liu 

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", ) != 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

2023-12-20 Thread Cédric Le Goater 

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.k...@gmail.com/

Reported-by: Khem Raj 
Signed-off-by: Cédric Le Goater 



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", ) != 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

2023-12-20 Thread Zhao Liu 
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 
> 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.k...@gmail.com/
> 
> Reported-by: Khem Raj 
> Signed-off-by: Cédric Le Goater 
> ---

Reviewed-by: Zhao Liu 

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", ) != 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

2023-12-20 Thread Eric Auger 
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.k...@gmail.com/
>
> Reported-by: Khem Raj 
> Signed-off-by: Cédric Le Goater 
Reviewed-by: Eric Auger 

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", ) != 1) {
>  error_setg_errno(errp, errno, "failed to read %s", group_path);
>  return -errno;

[PATCH] vfio/container: Replace basename with g_path_get_basename

2023-12-20 Thread Cédric Le Goater 
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.k...@gmail.com/

Reported-by: Khem Raj 
Signed-off-by: Cédric Le Goater 
---
 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", ) != 1) {
 error_setg_errno(errp, errno, "failed to read %s", group_path);
 return -errno;
-- 
2.43.0