[libvirt] [PATCH 13/23] src: replace last_component() with g_path_get_basename()

[Daniel P. Berrangé]

On Thu, Jan 02, 2020 at 05:42:30PM +0100, Fabiano Fidêncio wrote:
> [snip]
> 
> > diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
> > index f072afe857..16a527e399 100644
> > --- a/src/rpc/virnetsocket.c
> > +++ b/src/rpc/virnetsocket.c
> > @@ -55,7 +55,6 @@
> >  #include "virprobe.h"
> >  #include "virprocess.h"
> >  #include "virstring.h"
> > -#include "dirname.h"
> >  #include "passfd.h"
> >
> >  #if WITH_SSH2
> > @@ -668,7 +667,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> >      remoteAddr.len = sizeof(remoteAddr.data.un);
> >
> >      if (spawnDaemon) {
> > -        const char *binname;
> > +        g_autofree char *binname = NULL;
> >
> >          if (spawnDaemon && !binary) {
> >              virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
> > @@ -677,7 +676,7 @@ int virNetSocketNewConnectUNIX(const char *path,
> >              goto cleanup;
> >          }
> >
> > -        if (!(binname = last_component(binary)) || binname[0] == '\0') {
> > +        if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {
> 
> IIUC, this check is no longer valid.
> According to the g_path_get_basename() documentation "If file_name
> ends with a directory separator it gets the component before the last
> slash. If file_name consists only of directory separators (and on
> Windows, possibly a drive letter), a single separator is returned. If
> file_name is empty, it gets "."."
> 
> Knowing that, shouldn't we adapt the check accordingly?

Yes,

-        if (!(binname = g_path_get_basename(binary)) || binname[0] == '\0') {
-            virReportError(VIR_ERR_INTERNAL_ERROR,
-                           _("Cannot determine basename for binary '%s'"),
-                           binary);
-            goto cleanup;
-        }
-
+        binname = g_path_get_basename(binary);


> 
> [snip]
> 
> > diff --git a/src/util/virmdev.c b/src/util/virmdev.c
> > index cd52a91ffd..c2499c0a20 100644
> > --- a/src/util/virmdev.c
> > +++ b/src/util/virmdev.c
> > @@ -18,7 +18,6 @@
> >
> >  #include <config.h>
> >
> > -#include "dirname.h"
> >  #include "virmdev.h"
> >  #include "virlog.h"
> >  #include "virerror.h"
> > @@ -207,6 +206,7 @@ char *
> >  virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> >  {
> >      g_autofree char *result_path = NULL;
> > +    g_autofree char *result_file = NULL;
> >      g_autofree char *iommu_path = NULL;
> >      g_autofree char *dev_path = virMediatedDeviceGetSysfsPath(uuidstr);
> >      char *vfio_path = NULL;
> > @@ -226,7 +226,9 @@ virMediatedDeviceGetIOMMUGroupDev(const char *uuidstr)
> >          return NULL;
> >      }
> >
> > -    vfio_path = g_strdup_printf("/dev/vfio/%s", last_component(result_path));
> > +    result_file = g_path_get_basename(result_path);
> > +
> > +    vfio_path = g_strdup_printf("/dev/vfio/%s", result_file);
> 
> Please, while changing it, use g_build_filename() instead of g_strdup_printf().

I'm going to leave this as is, as we have so much file name building
code already that just printfs.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|