Re: [PATCH v2 11/13] block/nfs: Use URI parsing code from glib

[Eric Blake]

On Fri, Apr 12, 2024 at 03:24:13PM +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/nfs.c | 110 ++++++++++++++++++++++++++--------------------------
>  1 file changed, 54 insertions(+), 56 deletions(-)
> 

>      }
> -    if (g_strcmp0(uri->scheme, "nfs") != 0) {
> +    if (!g_str_equal(g_uri_get_scheme(uri), "nfs")) {

Another case where we should be considering whether g_ascii_strcasecmp
is better, as a separate patch.

> -    for (i = 0; i < qp->n; i++) {
> -        uint64_t val;
> -        if (!qp->p[i].value) {
> -            error_setg(errp, "Value for NFS parameter expected: %s",
> -                       qp->p[i].name);
> -            goto out;
> -        }
> -        if (parse_uint_full(qp->p[i].value, 0, &val)) {
> -            error_setg(errp, "Illegal value for NFS parameter: %s",

Pre-existing,...

> +
> +    uri_query = g_uri_get_query(uri);
> +    if (uri_query) {
> +        g_uri_params_iter_init(&qp, uri_query, -1, "&", G_URI_PARAMS_NONE);
> +        while (g_uri_params_iter_next(&qp, &qp_name, &qp_value, &gerror)) {
> +            uint64_t val;
> +            if (!qp_name || gerror) {
> +                error_setg(errp, "Failed to parse NFS parameter");
> +                return -EINVAL;
> +            }
> +            if (!qp_value) {
> +                error_setg(errp, "Value for NFS parameter expected: %s",
> +                           qp_name);
> +                return -EINVAL;
> +            }
> +            if (parse_uint_full(qp_value, 0, &val)) {
> +                error_setg(errp, "Illegal value for NFS parameter: %s",

...but since we're touching it, I prefer 'Invalid' over 'Illegal' (any
error message implying that you broke a law when you passed in bad
data is a bit aggressive).  Not a show-stopper to leave it alone.

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization:  qemu.org | libguestfs.org