Re: [PATCH v2 09/13] block/gluster: Use URI parsing code from glib

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Apr 12, 2024 at 09:40:11AM -0500, Eric Blake wrote:
> On Fri, Apr 12, 2024 at 03:24:11PM +0200, Thomas Huth wrote:
> > Since version 2.66, glib has useful URI parsing functions, too.
> > Use those instead of the QEMU-internal ones to be finally able
> > to get rid of the latter.
> > 
> > Signed-off-by: Thomas Huth <thuth@redhat.com>
> > ---
> >  block/gluster.c | 71 ++++++++++++++++++++++++-------------------------
> >  1 file changed, 35 insertions(+), 36 deletions(-)
> > 
> > diff --git a/block/gluster.c b/block/gluster.c
> > index cc74af06dc..1c9505f8bb 100644
> > --- a/block/gluster.c
> > +++ b/block/gluster.c
> > @@ -17,7 +17,6 @@
> >  #include "qapi/error.h"
> >  #include "qapi/qmp/qdict.h"
> >  #include "qapi/qmp/qerror.h"
> > -#include "qemu/uri.h"
> >  #include "qemu/error-report.h"
> >  #include "qemu/module.h"
> >  #include "qemu/option.h"
> > @@ -289,9 +288,9 @@ static void glfs_clear_preopened(glfs_t *fs)
> >      }
> >  }
> >  
> > -static int parse_volume_options(BlockdevOptionsGluster *gconf, char *path)
> > +static int parse_volume_options(BlockdevOptionsGluster *gconf, const char *path)
> 
> Is it worth mentioning in the commit message that this includes a
> const-correctness tweak?
> 
> > @@ -364,57 +363,57 @@ static int qemu_gluster_parse_uri(BlockdevOptionsGluster *gconf,
> >      QAPI_LIST_PREPEND(gconf->server, gsconf);
> >  
> >      /* transport */
> > -    if (!uri->scheme || !strcmp(uri->scheme, "gluster")) {
> > +    uri_scheme = g_uri_get_scheme(uri);
> > +    if (!uri_scheme || !strcmp(uri_scheme, "gluster")) {
> 
> Pre-existing, but per RFC 3986, we should probably be using strcasecmp
> for scheme comparisons (I'm not sure if g_uri_parse guarantees a
> lower-case return, even when the user passed in upper case).  That can
> be a separate patch.

Docs say it is lowercase:

  https://developer-old.gnome.org/glib/stable/glib-URI-Functions.html

  "on return, contains the scheme (converted to lowercase), or NULL."

With 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 :|