| Message ID | 20231220135302.932039-1-clg@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | vfio/container: Replace basename with g_path_get_basename | expand |
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;
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 > >
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;
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 >> >> >
>-----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
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;
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(-)