[PATCH 1/1] Replace AbsPath judgement method with g_path_is_absolute()

[Luke Yue]

On Wed, 2021-04-21 at 10:03 +0200, Michal Privoznik wrote:
> On 4/20/21 6:44 AM, Luke Yue wrote:
> > The g_path_is_absolute() considers more situations
> > than just a simply "path[0] == '/'".
> > 
> > Related issue: https://gitlab.com/libvirt/libvirt/-/issues/12
> > 
> > Signed-off-by: Luke Yue <lukedyue at gmail.com>
> > ---
> >   src/conf/backup_conf.c            | 2 +-
> >   src/conf/snapshot_conf.c          | 2 +-
> >   src/conf/storage_source_conf.c    | 2 +-
> >   src/lxc/lxc_native.c              | 2 +-
> >   src/qemu/qemu_block.c             | 2 +-
> >   src/storage_file/storage_source.c | 2 +-
> >   src/util/vircommand.c             | 2 +-
> >   src/vbox/vbox_snapshot_conf.c     | 2 +-
> >   src/vmware/vmware_conf.c          | 2 +-
> >   src/vmware/vmware_driver.c        | 2 +-
> >   tests/virtestmock.c               | 2 +-
> >   tools/virt-login-shell-helper.c   | 2 +-
> >   12 files changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/conf/backup_conf.c b/src/conf/backup_conf.c
> > index 2de77a59c0..7f54a25ff6 100644
> > --- a/src/conf/backup_conf.c
> > +++ b/src/conf/backup_conf.c
> > @@ -262,7 +262,7 @@ virDomainBackupDefParse(xmlXPathContextPtr
> > ctxt,
> >           }
> >   
> >           if (def->server->transport ==
> > VIR_STORAGE_NET_HOST_TRANS_UNIX &&
> > -            def->server->socket[0] != '/') {
> > +            !g_path_is_absolute(def->server->socket)) {
> >               virReportError(VIR_ERR_XML_ERROR,
> >                              _("backup socket path '%s' must be
> > absolute"),
> >                              def->server->socket);
> > diff --git a/src/conf/snapshot_conf.c b/src/conf/snapshot_conf.c
> > index 07c9ea7af7..df3f2a4c63 100644
> > --- a/src/conf/snapshot_conf.c
> > +++ b/src/conf/snapshot_conf.c
> > @@ -363,7 +363,7 @@ virDomainSnapshotDefParse(xmlXPathContextPtr
> > ctxt,
> >       def->file = g_steal_pointer(&memoryFile);
> >   
> >       /* verify that memory path is absolute */
> > -    if (def->file && def->file[0] != '/') {
> > +    if (def->file && !g_path_is_absolute(def->file)) {
> >           virReportError(VIR_ERR_XML_ERROR,
> >                          _("memory snapshot file path (%s) must be
> > absolute"),
> >                          def->file);
> > diff --git a/src/conf/storage_source_conf.c
> > b/src/conf/storage_source_conf.c
> > index 05939181d6..5ca06fa30a 100644
> > --- a/src/conf/storage_source_conf.c
> > +++ b/src/conf/storage_source_conf.c
> > @@ -1213,7 +1213,7 @@ virStorageSourceIsRelative(virStorageSource
> > *src)
> >       case VIR_STORAGE_TYPE_FILE:
> >       case VIR_STORAGE_TYPE_BLOCK:
> >       case VIR_STORAGE_TYPE_DIR:
> > -        return src->path[0] != '/';
> > +        return !g_path_is_absolute(src->path);
> >   
> >       case VIR_STORAGE_TYPE_NETWORK:
> >       case VIR_STORAGE_TYPE_VOLUME:
> > diff --git a/src/lxc/lxc_native.c b/src/lxc/lxc_native.c
> > index 083fb50af7..4bdd960e23 100644
> > --- a/src/lxc/lxc_native.c
> > +++ b/src/lxc/lxc_native.c
> > @@ -261,7 +261,7 @@ lxcAddFstabLine(virDomainDef *def, lxcFstab
> > *fstab)
> >       if (!options)
> >           return -1;
> >   
> > -    if (fstab->dst[0] != '/') {
> > +    if (!g_path_is_absolute(fstab->dst)) {
> >           dst = g_strdup_printf("/%s", fstab->dst);
> >       } else {
> >           dst = g_strdup(fstab->dst);
> > diff --git a/src/qemu/qemu_block.c b/src/qemu/qemu_block.c
> > index a972e1e368..c815daf1e6 100644
> > --- a/src/qemu/qemu_block.c
> > +++ b/src/qemu/qemu_block.c
> > @@ -427,7 +427,7 @@ qemuBlockStorageSourceGetURI(virStorageSource
> > *src)
> >           if (src->volume) {
> >               uri->path = g_strdup_printf("/%s/%s", src->volume,
> > src->path);
> >           } else {
> > -            uri->path = g_strdup_printf("%s%s", src->path[0] ==
> > '/' ? "" : "/",
> > +            uri->path = g_strdup_printf("%s%s",
> > g_path_is_absolute(src->path) ? "" : "/",
> >                                           src->path);
> 
> There's a soft limit on the length of a line: 80 chars. This means
> that 
> if possible then the limit should be honoured. In this case, the 
> arguments can go on separate lines.
> 

Thanks for the review, I will configure my editor to break line
automatically or remind me the limit.

> >           }
> >       }
> 
> 
> I've found couple of other places which you didn't fix. I'll squash
> them 
> into your patch and push.
> 
> Reviewed-by: Michal Privoznik <mprivozn at redhat.com>
> 
> Michal
> 

I didn't fix some of them because I was not pretty sure whether they
should be changed, like "Resource partition '%s' must start with '/'"
or some places are for UNIX-like systems only, so I left them
unchanged. Now I realize that keep consistency in codes may be more
important. Thanks.

Zelong