[PATCH 0/5] util/uri: Simplify the code, remove unused functions

[PATCH 0/5] util/uri: Simplify the code, remove unused functions

[Thomas Huth]

The URI function uri_string_unescape() has some overlap with functions
from the glib, so we can simplify our code here quite a bit.
While at it, I also noticed that there are many unused functions in
here which we likely can drop nowadays (it's better to use the functions
from glib anyway).

Thomas Huth (5):
  util/uri: Remove the unused "target" argument from
    uri_string_unescape()
  util/uri: Simplify uri_string_unescape()
  util/uri: Remove the uri_string_escape() function
  util/uri: Remove unused functions uri_resolve() and
    uri_resolve_relative()
  util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

 include/qemu/uri.h |   5 +-
 util/uri.c         | 843 +--------------------------------------------
 2 files changed, 16 insertions(+), 832 deletions(-)

-- 
2.43.0

[PATCH 1/5] util/uri: Remove the unused "target" argument from uri_string_unescape()

[Thomas Huth]

All callers pass NULL as target, so we can simplify the code by
dropping this parameter.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qemu/uri.h |  2 +-
 util/uri.c         | 32 ++++++++++++++------------------
 2 files changed, 15 insertions(+), 19 deletions(-)

diff --git a/include/qemu/uri.h b/include/qemu/uri.h
index 1855b764f2..aa54b6f251 100644
--- a/include/qemu/uri.h
+++ b/include/qemu/uri.h
@@ -79,7 +79,7 @@ URI *uri_parse_raw(const char *str, int raw);
 int uri_parse_into(URI *uri, const char *str);
 char *uri_to_string(URI *uri);
 char *uri_string_escape(const char *str, const char *list);
-char *uri_string_unescape(const char *str, int len, char *target);
+char *uri_string_unescape(const char *str, int len);
 void uri_free(URI *uri);
 
 /* Single web service query parameter 'name=value'. */
diff --git a/util/uri.c b/util/uri.c
index dcb3305236..33b6c7214e 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -267,7 +267,7 @@ static int rfc3986_parse_fragment(URI *uri, const char **str)
         if (uri->cleanup & 2) {
             uri->fragment = g_strndup(*str, cur - *str);
         } else {
-            uri->fragment = uri_string_unescape(*str, cur - *str, NULL);
+            uri->fragment = uri_string_unescape(*str, cur - *str);
         }
     }
     *str = cur;
@@ -368,7 +368,7 @@ static int rfc3986_parse_user_info(URI *uri, const char **str)
             if (uri->cleanup & 2) {
                 uri->user = g_strndup(*str, cur - *str);
             } else {
-                uri->user = uri_string_unescape(*str, cur - *str, NULL);
+                uri->user = uri_string_unescape(*str, cur - *str);
             }
         }
         *str = cur;
@@ -496,7 +496,7 @@ found:
             if (uri->cleanup & 2) {
                 uri->server = g_strndup(host, cur - host);
             } else {
-                uri->server = uri_string_unescape(host, cur - host, NULL);
+                uri->server = uri_string_unescape(host, cur - host);
             }
         } else {
             uri->server = NULL;
@@ -614,7 +614,7 @@ static int rfc3986_parse_path_ab_empty(URI *uri, const char **str)
             if (uri->cleanup & 2) {
                 uri->path = g_strndup(*str, cur - *str);
             } else {
-                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+                uri->path = uri_string_unescape(*str, cur - *str);
             }
         } else {
             uri->path = NULL;
@@ -663,7 +663,7 @@ static int rfc3986_parse_path_absolute(URI *uri, const char **str)
             if (uri->cleanup & 2) {
                 uri->path = g_strndup(*str, cur - *str);
             } else {
-                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+                uri->path = uri_string_unescape(*str, cur - *str);
             }
         } else {
             uri->path = NULL;
@@ -709,7 +709,7 @@ static int rfc3986_parse_path_rootless(URI *uri, const char **str)
             if (uri->cleanup & 2) {
                 uri->path = g_strndup(*str, cur - *str);
             } else {
-                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+                uri->path = uri_string_unescape(*str, cur - *str);
             }
         } else {
             uri->path = NULL;
@@ -755,7 +755,7 @@ static int rfc3986_parse_path_no_scheme(URI *uri, const char **str)
             if (uri->cleanup & 2) {
                 uri->path = g_strndup(*str, cur - *str);
             } else {
-                uri->path = uri_string_unescape(*str, cur - *str, NULL);
+                uri->path = uri_string_unescape(*str, cur - *str);
             }
         } else {
             uri->path = NULL;
@@ -1574,7 +1574,6 @@ static int is_hex(char c)
  * uri_string_unescape:
  * @str:  the string to unescape
  * @len:   the length in bytes to unescape (or <= 0 to indicate full string)
- * @target:  optional destination buffer
  *
  * Unescaping routine, but does not check that the string is an URI. The
  * output is a direct unsigned char translation of %XX values (no encoding)
@@ -1584,7 +1583,7 @@ static int is_hex(char c)
  * Returns a copy of the string, but unescaped, will return NULL only in case
  * of error
  */
-char *uri_string_unescape(const char *str, int len, char *target)
+char *uri_string_unescape(const char *str, int len)
 {
     char *ret, *out;
     const char *in;
@@ -1599,11 +1598,8 @@ char *uri_string_unescape(const char *str, int len, char *target)
         return NULL;
     }
 
-    if (target == NULL) {
-        ret = g_malloc(len + 1);
-    } else {
-        ret = target;
-    }
+    ret = g_malloc(len + 1);
+
     in = str;
     out = ret;
     while (len > 0) {
@@ -2274,14 +2270,14 @@ struct QueryParams *query_params_parse(const char *query)
          * and consistent with CGI.pm we assume value is "".
          */
         else if (!eq) {
-            name = uri_string_unescape(query, end - query, NULL);
+            name = uri_string_unescape(query, end - query);
             value = NULL;
         }
         /* Or if we have "name=" here (works around annoying
          * problem when calling uri_string_unescape with len = 0).
          */
         else if (eq + 1 == end) {
-            name = uri_string_unescape(query, eq - query, NULL);
+            name = uri_string_unescape(query, eq - query);
             value = g_new0(char, 1);
         }
         /* If the '=' character is at the beginning then we have
@@ -2293,8 +2289,8 @@ struct QueryParams *query_params_parse(const char *query)
 
         /* Otherwise it's "name=value". */
         else {
-            name = uri_string_unescape(query, eq - query, NULL);
-            value = uri_string_unescape(eq + 1, end - (eq + 1), NULL);
+            name = uri_string_unescape(query, eq - query);
+            value = uri_string_unescape(eq + 1, end - (eq + 1));
         }
 
         /* Append to the parameter set. */
-- 
2.43.0

[PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Thomas Huth]

uri_string_unescape() basically does the same as the glib function
g_uri_unescape_string(), with just an additional length parameter.
So we can simplify this function a lot by limiting the length with
g_strndup() first and then by calling g_uri_unescape_string() instead
of walking through the string manually.

Suggested-by: Stefan Weil <stefan.weil@weilnetz.de>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 util/uri.c | 49 +++----------------------------------------------
 1 file changed, 3 insertions(+), 46 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index 33b6c7214e..2a75f535ba 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1561,15 +1561,6 @@ done_cd:
     return 0;
 }
 
-static int is_hex(char c)
-{
-    if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
-        ((c >= 'A') && (c <= 'F'))) {
-        return 1;
-    }
-    return 0;
-}
-
 /**
  * uri_string_unescape:
  * @str:  the string to unescape
@@ -1585,8 +1576,7 @@ static int is_hex(char c)
  */
 char *uri_string_unescape(const char *str, int len)
 {
-    char *ret, *out;
-    const char *in;
+    g_autofree char *lstr = NULL;
 
     if (str == NULL) {
         return NULL;
@@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
     if (len <= 0) {
         len = strlen(str);
     }
-    if (len < 0) {
-        return NULL;
-    }
-
-    ret = g_malloc(len + 1);
+    lstr = g_strndup(str, len);
 
-    in = str;
-    out = ret;
-    while (len > 0) {
-        if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
-            in++;
-            if ((*in >= '0') && (*in <= '9')) {
-                *out = (*in - '0');
-            } else if ((*in >= 'a') && (*in <= 'f')) {
-                *out = (*in - 'a') + 10;
-            } else if ((*in >= 'A') && (*in <= 'F')) {
-                *out = (*in - 'A') + 10;
-            }
-            in++;
-            if ((*in >= '0') && (*in <= '9')) {
-                *out = *out * 16 + (*in - '0');
-            } else if ((*in >= 'a') && (*in <= 'f')) {
-                *out = *out * 16 + (*in - 'a') + 10;
-            } else if ((*in >= 'A') && (*in <= 'F')) {
-                *out = *out * 16 + (*in - 'A') + 10;
-            }
-            in++;
-            len -= 3;
-            out++;
-        } else {
-            *out++ = *in++;
-            len--;
-        }
-    }
-    *out = 0;
-    return ret;
+    return g_uri_unescape_string(lstr, NULL);
 }
 
 /**
-- 
2.43.0

[PATCH 3/5] util/uri: Remove the uri_string_escape() function

[Thomas Huth]

It is not used in QEMU - and if somebody needs this functionality,
they can simply use g_uri_escape_string() from the glib instead.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qemu/uri.h |  1 -
 util/uri.c         | 64 ----------------------------------------------
 2 files changed, 65 deletions(-)

diff --git a/include/qemu/uri.h b/include/qemu/uri.h
index aa54b6f251..c1734d28c3 100644
--- a/include/qemu/uri.h
+++ b/include/qemu/uri.h
@@ -78,7 +78,6 @@ URI *uri_parse(const char *str);
 URI *uri_parse_raw(const char *str, int raw);
 int uri_parse_into(URI *uri, const char *str);
 char *uri_to_string(URI *uri);
-char *uri_string_escape(const char *str, const char *list);
 char *uri_string_unescape(const char *str, int len);
 void uri_free(URI *uri);
 
diff --git a/util/uri.c b/util/uri.c
index 2a75f535ba..912e406523 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1589,70 +1589,6 @@ char *uri_string_unescape(const char *str, int len)
     return g_uri_unescape_string(lstr, NULL);
 }
 
-/**
- * uri_string_escape:
- * @str:  string to escape
- * @list: exception list string of chars not to escape
- *
- * This routine escapes a string to hex, ignoring reserved characters (a-z)
- * and the characters in the exception list.
- *
- * Returns a new escaped string or NULL in case of error.
- */
-char *uri_string_escape(const char *str, const char *list)
-{
-    char *ret, ch;
-    char *temp;
-    const char *in;
-    int len, out;
-
-    if (str == NULL) {
-        return NULL;
-    }
-    if (str[0] == 0) {
-        return g_strdup(str);
-    }
-    len = strlen(str);
-    if (!(len > 0)) {
-        return NULL;
-    }
-
-    len += 20;
-    ret = g_malloc(len);
-    in = str;
-    out = 0;
-    while (*in != 0) {
-        if (len - out <= 3) {
-            temp = realloc2n(ret, &len);
-            ret = temp;
-        }
-
-        ch = *in;
-
-        if ((ch != '@') && (!IS_UNRESERVED(ch)) && (!strchr(list, ch))) {
-            unsigned char val;
-            ret[out++] = '%';
-            val = ch >> 4;
-            if (val <= 9) {
-                ret[out++] = '0' + val;
-            } else {
-                ret[out++] = 'A' + val - 0xA;
-            }
-            val = ch & 0xF;
-            if (val <= 9) {
-                ret[out++] = '0' + val;
-            } else {
-                ret[out++] = 'A' + val - 0xA;
-            }
-            in++;
-        } else {
-            ret[out++] = *in++;
-        }
-    }
-    ret[out] = 0;
-    return ret;
-}
-
 /************************************************************************
  *                                                                      *
  *                           Public functions                           *
-- 
2.43.0

[PATCH 4/5] util/uri: Remove unused functions uri_resolve() and uri_resolve_relative()

[Thomas Huth]

These rather complex functions have never been used since they've been
introduced in 2012, so looks like they are not really useful for QEMU.
And since the static normalize_uri_path() function is also only used by
uri_resolve(), we can remove that function now, too.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 include/qemu/uri.h |   2 -
 util/uri.c         | 689 ---------------------------------------------
 2 files changed, 691 deletions(-)

diff --git a/include/qemu/uri.h b/include/qemu/uri.h
index c1734d28c3..93538b7578 100644
--- a/include/qemu/uri.h
+++ b/include/qemu/uri.h
@@ -72,8 +72,6 @@ typedef struct URI {
 } URI;
 
 URI *uri_new(void);
-char *uri_resolve(const char *URI, const char *base);
-char *uri_resolve_relative(const char *URI, const char *base);
 URI *uri_parse(const char *str);
 URI *uri_parse_raw(const char *str, int raw);
 int uri_parse_into(URI *uri, const char *str);
diff --git a/util/uri.c b/util/uri.c
index 912e406523..5f5ca79792 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -1355,212 +1355,6 @@ void uri_free(URI *uri)
  *                                                                      *
  ************************************************************************/
 
-/**
- * normalize_uri_path:
- * @path:  pointer to the path string
- *
- * Applies the 5 normalization steps to a path string--that is, RFC 2396
- * Section 5.2, steps 6.c through 6.g.
- *
- * Normalization occurs directly on the string, no new allocation is done
- *
- * Returns 0 or an error code
- */
-static int normalize_uri_path(char *path)
-{
-    char *cur, *out;
-
-    if (path == NULL) {
-        return -1;
-    }
-
-    /* Skip all initial "/" chars.  We want to get to the beginning of the
-     * first non-empty segment.
-     */
-    cur = path;
-    while (cur[0] == '/') {
-        ++cur;
-    }
-    if (cur[0] == '\0') {
-        return 0;
-    }
-
-    /* Keep everything we've seen so far.  */
-    out = cur;
-
-    /*
-     * Analyze each segment in sequence for cases (c) and (d).
-     */
-    while (cur[0] != '\0') {
-        /*
-         * c) All occurrences of "./", where "." is a complete path segment,
-         *    are removed from the buffer string.
-         */
-        if ((cur[0] == '.') && (cur[1] == '/')) {
-            cur += 2;
-            /* '//' normalization should be done at this point too */
-            while (cur[0] == '/') {
-                cur++;
-            }
-            continue;
-        }
-
-        /*
-         * d) If the buffer string ends with "." as a complete path segment,
-         *    that "." is removed.
-         */
-        if ((cur[0] == '.') && (cur[1] == '\0')) {
-            break;
-        }
-
-        /* Otherwise keep the segment.  */
-        while (cur[0] != '/') {
-            if (cur[0] == '\0') {
-                goto done_cd;
-            }
-            (out++)[0] = (cur++)[0];
-        }
-        /* nomalize // */
-        while ((cur[0] == '/') && (cur[1] == '/')) {
-            cur++;
-        }
-
-        (out++)[0] = (cur++)[0];
-    }
-done_cd:
-    out[0] = '\0';
-
-    /* Reset to the beginning of the first segment for the next sequence.  */
-    cur = path;
-    while (cur[0] == '/') {
-        ++cur;
-    }
-    if (cur[0] == '\0') {
-        return 0;
-    }
-
-    /*
-     * Analyze each segment in sequence for cases (e) and (f).
-     *
-     * e) All occurrences of "<segment>/../", where <segment> is a
-     *    complete path segment not equal to "..", are removed from the
-     *    buffer string.  Removal of these path segments is performed
-     *    iteratively, removing the leftmost matching pattern on each
-     *    iteration, until no matching pattern remains.
-     *
-     * f) If the buffer string ends with "<segment>/..", where <segment>
-     *    is a complete path segment not equal to "..", that
-     *    "<segment>/.." is removed.
-     *
-     * To satisfy the "iterative" clause in (e), we need to collapse the
-     * string every time we find something that needs to be removed.  Thus,
-     * we don't need to keep two pointers into the string: we only need a
-     * "current position" pointer.
-     */
-    while (1) {
-        char *segp, *tmp;
-
-        /* At the beginning of each iteration of this loop, "cur" points to
-         * the first character of the segment we want to examine.
-         */
-
-        /* Find the end of the current segment.  */
-        segp = cur;
-        while ((segp[0] != '/') && (segp[0] != '\0')) {
-            ++segp;
-        }
-
-        /* If this is the last segment, we're done (we need at least two
-         * segments to meet the criteria for the (e) and (f) cases).
-         */
-        if (segp[0] == '\0') {
-            break;
-        }
-
-        /* If the first segment is "..", or if the next segment _isn't_ "..",
-         * keep this segment and try the next one.
-         */
-        ++segp;
-        if (((cur[0] == '.') && (cur[1] == '.') && (segp == cur + 3)) ||
-            ((segp[0] != '.') || (segp[1] != '.') ||
-             ((segp[2] != '/') && (segp[2] != '\0')))) {
-            cur = segp;
-            continue;
-        }
-
-        /* If we get here, remove this segment and the next one and back up
-         * to the previous segment (if there is one), to implement the
-         * "iteratively" clause.  It's pretty much impossible to back up
-         * while maintaining two pointers into the buffer, so just compact
-         * the whole buffer now.
-         */
-
-        /* If this is the end of the buffer, we're done.  */
-        if (segp[2] == '\0') {
-            cur[0] = '\0';
-            break;
-        }
-        /* Valgrind complained, strcpy(cur, segp + 3); */
-        /* string will overlap, do not use strcpy */
-        tmp = cur;
-        segp += 3;
-        while ((*tmp++ = *segp++) != 0) {
-            /* No further work */
-        }
-
-        /* If there are no previous segments, then keep going from here.  */
-        segp = cur;
-        while ((segp > path) && ((--segp)[0] == '/')) {
-            /* No further work */
-        }
-        if (segp == path) {
-            continue;
-        }
-
-        /* "segp" is pointing to the end of a previous segment; find it's
-         * start.  We need to back up to the previous segment and start
-         * over with that to handle things like "foo/bar/../..".  If we
-         * don't do this, then on the first pass we'll remove the "bar/..",
-         * but be pointing at the second ".." so we won't realize we can also
-         * remove the "foo/..".
-         */
-        cur = segp;
-        while ((cur > path) && (cur[-1] != '/')) {
-            --cur;
-        }
-    }
-    out[0] = '\0';
-
-    /*
-     * g) If the resulting buffer string still begins with one or more
-     *    complete path segments of "..", then the reference is
-     *    considered to be in error. Implementations may handle this
-     *    error by retaining these components in the resolved path (i.e.,
-     *    treating them as part of the final URI), by removing them from
-     *    the resolved path (i.e., discarding relative levels above the
-     *    root), or by avoiding traversal of the reference.
-     *
-     * We discard them from the final path.
-     */
-    if (path[0] == '/') {
-        cur = path;
-        while ((cur[0] == '/') && (cur[1] == '.') && (cur[2] == '.') &&
-               ((cur[3] == '/') || (cur[3] == '\0'))) {
-            cur += 3;
-        }
-
-        if (cur != path) {
-            out = path;
-            while (cur[0] != '\0') {
-                (out++)[0] = (cur++)[0];
-            }
-            out[0] = 0;
-        }
-    }
-
-    return 0;
-}
-
 /**
  * uri_string_unescape:
  * @str:  the string to unescape
@@ -1595,489 +1389,6 @@ char *uri_string_unescape(const char *str, int len)
  *                                                                      *
  ************************************************************************/
 
-/**
- * uri_resolve:
- * @URI:  the URI instance found in the document
- * @base:  the base value
- *
- * Computes he final URI of the reference done by checking that
- * the given URI is valid, and building the final URI using the
- * base URI. This is processed according to section 5.2 of the
- * RFC 2396
- *
- * 5.2. Resolving Relative References to Absolute Form
- *
- * Returns a new URI string (to be freed by the caller) or NULL in case
- *         of error.
- */
-char *uri_resolve(const char *uri, const char *base)
-{
-    char *val = NULL;
-    int ret, len, indx, cur, out;
-    URI *ref = NULL;
-    URI *bas = NULL;
-    URI *res = NULL;
-
-    /*
-     * 1) The URI reference is parsed into the potential four components and
-     *    fragment identifier, as described in Section 4.3.
-     *
-     *    NOTE that a completely empty URI is treated by modern browsers
-     *    as a reference to "." rather than as a synonym for the current
-     *    URI.  Should we do that here?
-     */
-    if (uri == NULL) {
-        ret = -1;
-    } else {
-        if (*uri) {
-            ref = uri_new();
-            ret = uri_parse_into(ref, uri);
-        } else {
-            ret = 0;
-        }
-    }
-    if (ret != 0) {
-        goto done;
-    }
-    if ((ref != NULL) && (ref->scheme != NULL)) {
-        /*
-         * The URI is absolute don't modify.
-         */
-        val = g_strdup(uri);
-        goto done;
-    }
-    if (base == NULL) {
-        ret = -1;
-    } else {
-        bas = uri_new();
-        ret = uri_parse_into(bas, base);
-    }
-    if (ret != 0) {
-        if (ref) {
-            val = uri_to_string(ref);
-        }
-        goto done;
-    }
-    if (ref == NULL) {
-        /*
-         * the base fragment must be ignored
-         */
-        g_free(bas->fragment);
-        bas->fragment = NULL;
-        val = uri_to_string(bas);
-        goto done;
-    }
-
-    /*
-     * 2) If the path component is empty and the scheme, authority, and
-     *    query components are undefined, then it is a reference to the
-     *    current document and we are done.  Otherwise, the reference URI's
-     *    query and fragment components are defined as found (or not found)
-     *    within the URI reference and not inherited from the base URI.
-     *
-     *    NOTE that in modern browsers, the parsing differs from the above
-     *    in the following aspect:  the query component is allowed to be
-     *    defined while still treating this as a reference to the current
-     *    document.
-     */
-    res = uri_new();
-    if ((ref->scheme == NULL) && (ref->path == NULL) &&
-        ((ref->authority == NULL) && (ref->server == NULL))) {
-        res->scheme = g_strdup(bas->scheme);
-        if (bas->authority != NULL) {
-            res->authority = g_strdup(bas->authority);
-        } else if (bas->server != NULL) {
-            res->server = g_strdup(bas->server);
-            res->user = g_strdup(bas->user);
-            res->port = bas->port;
-        }
-        res->path = g_strdup(bas->path);
-        if (ref->query != NULL) {
-            res->query = g_strdup(ref->query);
-        } else {
-            res->query = g_strdup(bas->query);
-        }
-        res->fragment = g_strdup(ref->fragment);
-        goto step_7;
-    }
-
-    /*
-     * 3) If the scheme component is defined, indicating that the reference
-     *    starts with a scheme name, then the reference is interpreted as an
-     *    absolute URI and we are done.  Otherwise, the reference URI's
-     *    scheme is inherited from the base URI's scheme component.
-     */
-    if (ref->scheme != NULL) {
-        val = uri_to_string(ref);
-        goto done;
-    }
-    res->scheme = g_strdup(bas->scheme);
-
-    res->query = g_strdup(ref->query);
-    res->fragment = g_strdup(ref->fragment);
-
-    /*
-     * 4) If the authority component is defined, then the reference is a
-     *    network-path and we skip to step 7.  Otherwise, the reference
-     *    URI's authority is inherited from the base URI's authority
-     *    component, which will also be undefined if the URI scheme does not
-     *    use an authority component.
-     */
-    if ((ref->authority != NULL) || (ref->server != NULL)) {
-        if (ref->authority != NULL) {
-            res->authority = g_strdup(ref->authority);
-        } else {
-            res->server = g_strdup(ref->server);
-            res->user = g_strdup(ref->user);
-            res->port = ref->port;
-        }
-        res->path = g_strdup(ref->path);
-        goto step_7;
-    }
-    if (bas->authority != NULL) {
-        res->authority = g_strdup(bas->authority);
-    } else if (bas->server != NULL) {
-        res->server = g_strdup(bas->server);
-        res->user = g_strdup(bas->user);
-        res->port = bas->port;
-    }
-
-    /*
-     * 5) If the path component begins with a slash character ("/"), then
-     *    the reference is an absolute-path and we skip to step 7.
-     */
-    if ((ref->path != NULL) && (ref->path[0] == '/')) {
-        res->path = g_strdup(ref->path);
-        goto step_7;
-    }
-
-    /*
-     * 6) If this step is reached, then we are resolving a relative-path
-     *    reference.  The relative path needs to be merged with the base
-     *    URI's path.  Although there are many ways to do this, we will
-     *    describe a simple method using a separate string buffer.
-     *
-     * Allocate a buffer large enough for the result string.
-     */
-    len = 2; /* extra / and 0 */
-    if (ref->path != NULL) {
-        len += strlen(ref->path);
-    }
-    if (bas->path != NULL) {
-        len += strlen(bas->path);
-    }
-    res->path = g_malloc(len);
-    res->path[0] = 0;
-
-    /*
-     * a) All but the last segment of the base URI's path component is
-     *    copied to the buffer.  In other words, any characters after the
-     *    last (right-most) slash character, if any, are excluded.
-     */
-    cur = 0;
-    out = 0;
-    if (bas->path != NULL) {
-        while (bas->path[cur] != 0) {
-            while ((bas->path[cur] != 0) && (bas->path[cur] != '/')) {
-                cur++;
-            }
-            if (bas->path[cur] == 0) {
-                break;
-            }
-
-            cur++;
-            while (out < cur) {
-                res->path[out] = bas->path[out];
-                out++;
-            }
-        }
-    }
-    res->path[out] = 0;
-
-    /*
-     * b) The reference's path component is appended to the buffer
-     *    string.
-     */
-    if (ref->path != NULL && ref->path[0] != 0) {
-        indx = 0;
-        /*
-         * Ensure the path includes a '/'
-         */
-        if ((out == 0) && (bas->server != NULL)) {
-            res->path[out++] = '/';
-        }
-        while (ref->path[indx] != 0) {
-            res->path[out++] = ref->path[indx++];
-        }
-    }
-    res->path[out] = 0;
-
-    /*
-     * Steps c) to h) are really path normalization steps
-     */
-    normalize_uri_path(res->path);
-
-step_7:
-
-    /*
-     * 7) The resulting URI components, including any inherited from the
-     *    base URI, are recombined to give the absolute form of the URI
-     *    reference.
-     */
-    val = uri_to_string(res);
-
-done:
-    uri_free(ref);
-    uri_free(bas);
-    uri_free(res);
-    return val;
-}
-
-/**
- * uri_resolve_relative:
- * @URI:  the URI reference under consideration
- * @base:  the base value
- *
- * Expresses the URI of the reference in terms relative to the
- * base.  Some examples of this operation include:
- *     base = "http://site1.com/docs/book1.html"
- *        URI input                        URI returned
- *     docs/pic1.gif                    pic1.gif
- *     docs/img/pic1.gif                img/pic1.gif
- *     img/pic1.gif                     ../img/pic1.gif
- *     http://site1.com/docs/pic1.gif   pic1.gif
- *     http://site2.com/docs/pic1.gif   http://site2.com/docs/pic1.gif
- *
- *     base = "docs/book1.html"
- *        URI input                        URI returned
- *     docs/pic1.gif                    pic1.gif
- *     docs/img/pic1.gif                img/pic1.gif
- *     img/pic1.gif                     ../img/pic1.gif
- *     http://site1.com/docs/pic1.gif   http://site1.com/docs/pic1.gif
- *
- *
- * Note: if the URI reference is really weird or complicated, it may be
- *       worthwhile to first convert it into a "nice" one by calling
- *       uri_resolve (using 'base') before calling this routine,
- *       since this routine (for reasonable efficiency) assumes URI has
- *       already been through some validation.
- *
- * Returns a new URI string (to be freed by the caller) or NULL in case
- * error.
- */
-char *uri_resolve_relative(const char *uri, const char *base)
-{
-    char *val = NULL;
-    int ret;
-    int ix;
-    int pos = 0;
-    int nbslash = 0;
-    int len;
-    URI *ref = NULL;
-    URI *bas = NULL;
-    char *bptr, *uptr, *vptr;
-    int remove_path = 0;
-
-    if ((uri == NULL) || (*uri == 0)) {
-        return NULL;
-    }
-
-    /*
-     * First parse URI into a standard form
-     */
-    ref = uri_new();
-    /* If URI not already in "relative" form */
-    if (uri[0] != '.') {
-        ret = uri_parse_into(ref, uri);
-        if (ret != 0) {
-            goto done; /* Error in URI, return NULL */
-        }
-    } else {
-        ref->path = g_strdup(uri);
-    }
-
-    /*
-     * Next parse base into the same standard form
-     */
-    if ((base == NULL) || (*base == 0)) {
-        val = g_strdup(uri);
-        goto done;
-    }
-    bas = uri_new();
-    if (base[0] != '.') {
-        ret = uri_parse_into(bas, base);
-        if (ret != 0) {
-            goto done; /* Error in base, return NULL */
-        }
-    } else {
-        bas->path = g_strdup(base);
-    }
-
-    /*
-     * If the scheme / server on the URI differs from the base,
-     * just return the URI
-     */
-    if ((ref->scheme != NULL) &&
-        ((bas->scheme == NULL) || (strcmp(bas->scheme, ref->scheme)) ||
-         (strcmp(bas->server, ref->server)))) {
-        val = g_strdup(uri);
-        goto done;
-    }
-    if (bas->path == ref->path ||
-        (bas->path && ref->path && !strcmp(bas->path, ref->path))) {
-        val = g_strdup("");
-        goto done;
-    }
-    if (bas->path == NULL) {
-        val = g_strdup(ref->path);
-        goto done;
-    }
-    if (ref->path == NULL) {
-        ref->path = (char *)"/";
-        remove_path = 1;
-    }
-
-    /*
-     * At this point (at last!) we can compare the two paths
-     *
-     * First we take care of the special case where either of the
-     * two path components may be missing (bug 316224)
-     */
-    if (bas->path == NULL) {
-        if (ref->path != NULL) {
-            uptr = ref->path;
-            if (*uptr == '/') {
-                uptr++;
-            }
-            /* exception characters from uri_to_string */
-            val = uri_string_escape(uptr, "/;&=+$,");
-        }
-        goto done;
-    }
-    bptr = bas->path;
-    if (ref->path == NULL) {
-        for (ix = 0; bptr[ix] != 0; ix++) {
-            if (bptr[ix] == '/') {
-                nbslash++;
-            }
-        }
-        uptr = NULL;
-        len = 1; /* this is for a string terminator only */
-    } else {
-        /*
-         * Next we compare the two strings and find where they first differ
-         */
-        if ((ref->path[pos] == '.') && (ref->path[pos + 1] == '/')) {
-            pos += 2;
-        }
-        if ((*bptr == '.') && (bptr[1] == '/')) {
-            bptr += 2;
-        } else if ((*bptr == '/') && (ref->path[pos] != '/')) {
-            bptr++;
-        }
-        while ((bptr[pos] == ref->path[pos]) && (bptr[pos] != 0)) {
-            pos++;
-        }
-
-        if (bptr[pos] == ref->path[pos]) {
-            val = g_strdup("");
-            goto done; /* (I can't imagine why anyone would do this) */
-        }
-
-        /*
-         * In URI, "back up" to the last '/' encountered.  This will be the
-         * beginning of the "unique" suffix of URI
-         */
-        ix = pos;
-        if ((ref->path[ix] == '/') && (ix > 0)) {
-            ix--;
-        } else if ((ref->path[ix] == 0) && (ix > 1)
-                && (ref->path[ix - 1] == '/')) {
-            ix -= 2;
-        }
-        for (; ix > 0; ix--) {
-            if (ref->path[ix] == '/') {
-                break;
-            }
-        }
-        if (ix == 0) {
-            uptr = ref->path;
-        } else {
-            ix++;
-            uptr = &ref->path[ix];
-        }
-
-        /*
-         * In base, count the number of '/' from the differing point
-         */
-        if (bptr[pos] != ref->path[pos]) { /* check for trivial URI == base */
-            for (; bptr[ix] != 0; ix++) {
-                if (bptr[ix] == '/') {
-                    nbslash++;
-                }
-            }
-        }
-        len = strlen(uptr) + 1;
-    }
-
-    if (nbslash == 0) {
-        if (uptr != NULL) {
-            /* exception characters from uri_to_string */
-            val = uri_string_escape(uptr, "/;&=+$,");
-        }
-        goto done;
-    }
-
-    /*
-     * Allocate just enough space for the returned string -
-     * length of the remainder of the URI, plus enough space
-     * for the "../" groups, plus one for the terminator
-     */
-    val = g_malloc(len + 3 * nbslash);
-    vptr = val;
-    /*
-     * Put in as many "../" as needed
-     */
-    for (; nbslash > 0; nbslash--) {
-        *vptr++ = '.';
-        *vptr++ = '.';
-        *vptr++ = '/';
-    }
-    /*
-     * Finish up with the end of the URI
-     */
-    if (uptr != NULL) {
-        if ((vptr > val) && (len > 0) && (uptr[0] == '/') &&
-            (vptr[-1] == '/')) {
-            memcpy(vptr, uptr + 1, len - 1);
-            vptr[len - 2] = 0;
-        } else {
-            memcpy(vptr, uptr, len);
-            vptr[len - 1] = 0;
-        }
-    } else {
-        vptr[len - 1] = 0;
-    }
-
-    /* escape the freshly-built path */
-    vptr = val;
-    /* exception characters from uri_to_string */
-    val = uri_string_escape(vptr, "/;&=+$,");
-    g_free(vptr);
-
-done:
-    /*
-     * Free the working variables
-     */
-    if (remove_path != 0) {
-        ref->path = NULL;
-    }
-    uri_free(ref);
-    uri_free(bas);
-
-    return val;
-}
-
 /*
  * Utility functions to help parse and assemble query strings.
  */
-- 
2.43.0

[PATCH 5/5] util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

[Thomas Huth]

They are not used anywhere, so there's no need to keep them around.

Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 util/uri.c | 13 -------------
 1 file changed, 13 deletions(-)

diff --git a/util/uri.c b/util/uri.c
index 5f5ca79792..2deab91da3 100644
--- a/util/uri.c
+++ b/util/uri.c
@@ -163,19 +163,6 @@ static void uri_clean(URI *uri);
      ((*(p) == '+')) || ((*(p) == ',')) || ((*(p) == ';')) ||                  \
      ((*(p) == '=')) || ((*(p) == '\'')))
 
-/*
- *    gen-delims    = ":" / "/" / "?" / "#" / "[" / "]" / "@"
- */
-#define ISA_GEN_DELIM(p)                                                       \
-    (((*(p) == ':')) || ((*(p) == '/')) || ((*(p) == '?')) ||                  \
-     ((*(p) == '#')) || ((*(p) == '[')) || ((*(p) == ']')) ||                  \
-     ((*(p) == '@')))
-
-/*
- *    reserved      = gen-delims / sub-delims
- */
-#define ISA_RESERVED(p) (ISA_GEN_DELIM(p) || (ISA_SUB_DELIM(p)))
-
 /*
  *    unreserved    = ALPHA / DIGIT / "-" / "." / "_" / "~"
  */
-- 
2.43.0

Re: [PATCH 0/5] util/uri: Simplify the code, remove unused functions

[Daniel P. Berrangé]

On Mon, Jan 22, 2024 at 08:17:48PM +0100, Thomas Huth wrote:
> The URI function uri_string_unescape() has some overlap with functions
> from the glib, so we can simplify our code here quite a bit.
> While at it, I also noticed that there are many unused functions in
> here which we likely can drop nowadays (it's better to use the functions
> from glib anyway).
> 
> Thomas Huth (5):
>   util/uri: Remove the unused "target" argument from
>     uri_string_unescape()
>   util/uri: Simplify uri_string_unescape()
>   util/uri: Remove the uri_string_escape() function
>   util/uri: Remove unused functions uri_resolve() and
>     uri_resolve_relative()
>   util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()
> 
>  include/qemu/uri.h |   5 +-
>  util/uri.c         | 843 +--------------------------------------------
>  2 files changed, 16 insertions(+), 832 deletions(-)

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>


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

Re: [PATCH 1/5] util/uri: Remove the unused "target" argument from uri_string_unescape()

[Stefan Weil]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> All callers pass NULL as target, so we can simplify the code by
> dropping this parameter.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |  2 +-
>   util/uri.c         | 32 ++++++++++++++------------------
>   2 files changed, 15 insertions(+), 19 deletions(-)


Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 1/5] util/uri: Remove the unused "target" argument from uri_string_unescape()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> All callers pass NULL as target, so we can simplify the code by
> dropping this parameter.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |  2 +-
>   util/uri.c         | 32 ++++++++++++++------------------
>   2 files changed, 15 insertions(+), 19 deletions(-)


Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 3/5] util/uri: Remove the uri_string_escape() function

[Stefan Weil]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> It is not used in QEMU - and if somebody needs this functionality,
> they can simply use g_uri_escape_string() from the glib instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |  1 -
>   util/uri.c         | 64 ----------------------------------------------
>   2 files changed, 65 deletions(-)


The removed function is used in util/uri.c, so this patch breaks the 
compilation.

That can be fixed by applying patch 4 before this one.

With that re-ordering you may add my signature:

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 3/5] util/uri: Remove the uri_string_escape() function

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> It is not used in QEMU - and if somebody needs this functionality,
> they can simply use g_uri_escape_string() from the glib instead.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |  1 -
>   util/uri.c         | 64 ----------------------------------------------
>   2 files changed, 65 deletions(-)


The removed function is used in util/uri.c, so this patch breaks the 
compilation.

That can be fixed by applying patch 4 before this one.

With that re-ordering you may add my signature:

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 4/5] util/uri: Remove unused functions uri_resolve() and uri_resolve_relative()

[Stefan Weil]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> These rather complex functions have never been used since they've been
> introduced in 2012, so looks like they are not really useful for QEMU.
> And since the static normalize_uri_path() function is also only used by
> uri_resolve(), we can remove that function now, too.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |   2 -
>   util/uri.c         | 689 ---------------------------------------------
>   2 files changed, 691 deletions(-)
>

This patch should be applied before patch 3 (so switch the order of the 
patches 3 and 4). With this change:

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 4/5] util/uri: Remove unused functions uri_resolve() and uri_resolve_relative()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> These rather complex functions have never been used since they've been
> introduced in 2012, so looks like they are not really useful for QEMU.
> And since the static normalize_uri_path() function is also only used by
> uri_resolve(), we can remove that function now, too.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |   2 -
>   util/uri.c         | 689 ---------------------------------------------
>   2 files changed, 691 deletions(-)
>

This patch should be applied before patch 3 (so switch the order of the 
patches 3 and 4). With this change:

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 5/5] util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

[Stefan Weil]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> They are not used anywhere, so there's no need to keep them around.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   util/uri.c | 13 -------------
>   1 file changed, 13 deletions(-)
>

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 5/5] util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

[Stefan Weil via]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> They are not used anywhere, so there's no need to keep them around.
>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   util/uri.c | 13 -------------
>   1 file changed, 13 deletions(-)
>

Reviewed-by: Stefan Weil <sw@weilnetz.de>

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Stefan Weil]

[-- Attachment #1: Type: text/plain, Size: 2876 bytes --]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> uri_string_unescape() basically does the same as the glib function
> g_uri_unescape_string(), with just an additional length parameter.
> So we can simplify this function a lot by limiting the length with
> g_strndup() first and then by calling g_uri_unescape_string() instead
> of walking through the string manually.
>
> Suggested-by: Stefan Weil<stefan.weil@weilnetz.de>

Can my e-mail address be replaced by another one (sw@weilnetz.de)?

> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   util/uri.c | 49 +++----------------------------------------------
>   1 file changed, 3 insertions(+), 46 deletions(-)
>
> diff --git a/util/uri.c b/util/uri.c
> index 33b6c7214e..2a75f535ba 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1561,15 +1561,6 @@ done_cd:
>       return 0;
>   }
>   
> -static int is_hex(char c)
> -{
> -    if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
> -        ((c >= 'A') && (c <= 'F'))) {
> -        return 1;
> -    }
> -    return 0;
> -}
> -
>   /**
>    * uri_string_unescape:
>    * @str:  the string to unescape
> @@ -1585,8 +1576,7 @@ static int is_hex(char c)
>    */
>   char *uri_string_unescape(const char *str, int len)
>   {
> -    char *ret, *out;
> -    const char *in;
> +    g_autofree char *lstr = NULL;


Is it necessary to assign NULL? It does not look so.


>   
>       if (str == NULL) {
>           return NULL;
> @@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
>       if (len <= 0) {
>           len = strlen(str);
>       }
> -    if (len < 0) {
> -        return NULL;
> -    }
> -
> -    ret = g_malloc(len + 1);
> +    lstr = g_strndup(str, len);
>   
> -    in = str;
> -    out = ret;
> -    while (len > 0) {
> -        if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = (*in - 'A') + 10;
> -            }
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = *out * 16 + (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = *out * 16 + (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = *out * 16 + (*in - 'A') + 10;
> -            }
> -            in++;
> -            len -= 3;
> -            out++;
> -        } else {
> -            *out++ = *in++;
> -            len--;
> -        }
> -    }
> -    *out = 0;
> -    return ret;
> +    return g_uri_unescape_string(lstr, NULL);
>   }
>   
>   /**


Thank you.

Reviewed-by: Stefan Weil <sw@weilnetz.de>


[-- Attachment #2: Type: text/html, Size: 3986 bytes --]

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Stefan Weil via]

[-- Attachment #1: Type: text/plain, Size: 2876 bytes --]

Am 22.01.24 um 20:17 schrieb Thomas Huth:

> uri_string_unescape() basically does the same as the glib function
> g_uri_unescape_string(), with just an additional length parameter.
> So we can simplify this function a lot by limiting the length with
> g_strndup() first and then by calling g_uri_unescape_string() instead
> of walking through the string manually.
>
> Suggested-by: Stefan Weil<stefan.weil@weilnetz.de>

Can my e-mail address be replaced by another one (sw@weilnetz.de)?

> Signed-off-by: Thomas Huth<thuth@redhat.com>
> ---
>   util/uri.c | 49 +++----------------------------------------------
>   1 file changed, 3 insertions(+), 46 deletions(-)
>
> diff --git a/util/uri.c b/util/uri.c
> index 33b6c7214e..2a75f535ba 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1561,15 +1561,6 @@ done_cd:
>       return 0;
>   }
>   
> -static int is_hex(char c)
> -{
> -    if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
> -        ((c >= 'A') && (c <= 'F'))) {
> -        return 1;
> -    }
> -    return 0;
> -}
> -
>   /**
>    * uri_string_unescape:
>    * @str:  the string to unescape
> @@ -1585,8 +1576,7 @@ static int is_hex(char c)
>    */
>   char *uri_string_unescape(const char *str, int len)
>   {
> -    char *ret, *out;
> -    const char *in;
> +    g_autofree char *lstr = NULL;


Is it necessary to assign NULL? It does not look so.


>   
>       if (str == NULL) {
>           return NULL;
> @@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
>       if (len <= 0) {
>           len = strlen(str);
>       }
> -    if (len < 0) {
> -        return NULL;
> -    }
> -
> -    ret = g_malloc(len + 1);
> +    lstr = g_strndup(str, len);
>   
> -    in = str;
> -    out = ret;
> -    while (len > 0) {
> -        if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = (*in - 'A') + 10;
> -            }
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = *out * 16 + (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = *out * 16 + (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = *out * 16 + (*in - 'A') + 10;
> -            }
> -            in++;
> -            len -= 3;
> -            out++;
> -        } else {
> -            *out++ = *in++;
> -            len--;
> -        }
> -    }
> -    *out = 0;
> -    return ret;
> +    return g_uri_unescape_string(lstr, NULL);
>   }
>   
>   /**


Thank you.

Reviewed-by: Stefan Weil <sw@weilnetz.de>


[-- Attachment #2: Type: text/html, Size: 3986 bytes --]

Re: [PATCH 1/5] util/uri: Remove the unused "target" argument from uri_string_unescape()

[Philippe Mathieu-Daudé]

On 22/1/24 20:17, Thomas Huth wrote:
> All callers pass NULL as target, so we can simplify the code by
> dropping this parameter.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   include/qemu/uri.h |  2 +-
>   util/uri.c         | 32 ++++++++++++++------------------
>   2 files changed, 15 insertions(+), 19 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

Re: [PATCH 5/5] util/uri: Remove unused macros ISA_RESERVED() and ISA_GEN_DELIM()

[Philippe Mathieu-Daudé]

On 22/1/24 20:17, Thomas Huth wrote:
> They are not used anywhere, so there's no need to keep them around.
> 
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>   util/uri.c | 13 -------------
>   1 file changed, 13 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>

(could be reordered as patch 1 to clarify it is not
  an effect of the uri_string_foo() cleanups)

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Thomas Huth]

On 22/01/2024 22.22, Stefan Weil wrote:
> Am 22.01.24 um 20:17 schrieb Thomas Huth:
> 
>> uri_string_unescape() basically does the same as the glib function
>> g_uri_unescape_string(), with just an additional length parameter.
>> So we can simplify this function a lot by limiting the length with
>> g_strndup() first and then by calling g_uri_unescape_string() instead
>> of walking through the string manually.
>>
>> Suggested-by: Stefan Weil<stefan.weil@weilnetz.de>
> 
> Can my e-mail address be replaced by another one (sw@weilnetz.de)?

Sure! ... not sure where I copy-n-pasted the other one from ... sorry for that.

>> @@ -1585,8 +1576,7 @@ static int is_hex(char c)
>>    */
>>   char *uri_string_unescape(const char *str, int len)
>>   {
>> -    char *ret, *out;
>> -    const char *in;
>> +    g_autofree char *lstr = NULL;
> 
> 
> Is it necessary to assign NULL? It does not look so.

Yes, it's necessary for the early "return NULL" statement below. Since it's 
an g_autofree variable, it must either be set to a valid allocated buffer or 
NULL before returning.

>>   
>>       if (str == NULL) {
>>           return NULL;
>> @@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
>>       if (len <= 0) {
>>           len = strlen(str);
>>       }
>> -    if (len < 0) {
>> -        return NULL;
>> -    }
>> -
>> -    ret = g_malloc(len + 1);
>> +    lstr = g_strndup(str, len);
>>   
>> -    in = str;
>> -    out = ret;
>> -    while (len > 0) {
>> -        if ((len > 2) && (*in == '%') && (is_hex(in[1])) && (is_hex(in[2]))) {
>> -            in++;
>> -            if ((*in >= '0') && (*in <= '9')) {
>> -                *out = (*in - '0');
>> -            } else if ((*in >= 'a') && (*in <= 'f')) {
>> -                *out = (*in - 'a') + 10;
>> -            } else if ((*in >= 'A') && (*in <= 'F')) {
>> -                *out = (*in - 'A') + 10;
>> -            }
>> -            in++;
>> -            if ((*in >= '0') && (*in <= '9')) {
>> -                *out = *out * 16 + (*in - '0');
>> -            } else if ((*in >= 'a') && (*in <= 'f')) {
>> -                *out = *out * 16 + (*in - 'a') + 10;
>> -            } else if ((*in >= 'A') && (*in <= 'F')) {
>> -                *out = *out * 16 + (*in - 'A') + 10;
>> -            }
>> -            in++;
>> -            len -= 3;
>> -            out++;
>> -        } else {
>> -            *out++ = *in++;
>> -            len--;
>> -        }
>> -    }
>> -    *out = 0;
>> -    return ret;
>> +    return g_uri_unescape_string(lstr, NULL);
>>   }
>>   
>>   /**
> 
> 
> Thank you.
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

Thanks!

  Thomas

Re: [PATCH 3/5] util/uri: Remove the uri_string_escape() function

[Thomas Huth]

On 22/01/2024 21.59, Stefan Weil wrote:
> Am 22.01.24 um 20:17 schrieb Thomas Huth:
> 
>> It is not used in QEMU - and if somebody needs this functionality,
>> they can simply use g_uri_escape_string() from the glib instead.
>>
>> Signed-off-by: Thomas Huth <thuth@redhat.com>
>> ---
>>   include/qemu/uri.h |  1 -
>>   util/uri.c         | 64 ----------------------------------------------
>>   2 files changed, 65 deletions(-)
> 
> 
> The removed function is used in util/uri.c, so this patch breaks the 
> compilation.
> 
> That can be fixed by applying patch 4 before this one.
> 
> With that re-ordering you may add my signature:
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

D'oh, I originally developed the patches the other way round indeed, but 
then thought it would be nicer for review this way and swapped the order 
without checking :-( ... I'll swap it back again.

Thanks!

  Thomas

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Paolo Bonzini]

[-- Attachment #1: Type: text/plain, Size: 2781 bytes --]

Il lun 22 gen 2024, 20:18 Thomas Huth <thuth@redhat.com> ha scritto:

> uri_string_unescape() basically does the same as the glib function
> g_uri_unescape_string(), with just an additional length parameter.
>

You can replace it altogether with g_uri_unescape_segment.

Paolo

So we can simplify this function a lot by limiting the length with
> g_strndup() first and then by calling g_uri_unescape_string() instead
> of walking through the string manually.
>
> Suggested-by: Stefan Weil <stefan.weil@weilnetz.de>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  util/uri.c | 49 +++----------------------------------------------
>  1 file changed, 3 insertions(+), 46 deletions(-)
>
> diff --git a/util/uri.c b/util/uri.c
> index 33b6c7214e..2a75f535ba 100644
> --- a/util/uri.c
> +++ b/util/uri.c
> @@ -1561,15 +1561,6 @@ done_cd:
>      return 0;
>  }
>
> -static int is_hex(char c)
> -{
> -    if (((c >= '0') && (c <= '9')) || ((c >= 'a') && (c <= 'f')) ||
> -        ((c >= 'A') && (c <= 'F'))) {
> -        return 1;
> -    }
> -    return 0;
> -}
> -
>  /**
>   * uri_string_unescape:
>   * @str:  the string to unescape
> @@ -1585,8 +1576,7 @@ static int is_hex(char c)
>   */
>  char *uri_string_unescape(const char *str, int len)
>  {
> -    char *ret, *out;
> -    const char *in;
> +    g_autofree char *lstr = NULL;
>
>      if (str == NULL) {
>          return NULL;
> @@ -1594,42 +1584,9 @@ char *uri_string_unescape(const char *str, int len)
>      if (len <= 0) {
>          len = strlen(str);
>      }
> -    if (len < 0) {
> -        return NULL;
> -    }
> -
> -    ret = g_malloc(len + 1);
> +    lstr = g_strndup(str, len);
>
> -    in = str;
> -    out = ret;
> -    while (len > 0) {
> -        if ((len > 2) && (*in == '%') && (is_hex(in[1])) &&
> (is_hex(in[2]))) {
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = (*in - 'A') + 10;
> -            }
> -            in++;
> -            if ((*in >= '0') && (*in <= '9')) {
> -                *out = *out * 16 + (*in - '0');
> -            } else if ((*in >= 'a') && (*in <= 'f')) {
> -                *out = *out * 16 + (*in - 'a') + 10;
> -            } else if ((*in >= 'A') && (*in <= 'F')) {
> -                *out = *out * 16 + (*in - 'A') + 10;
> -            }
> -            in++;
> -            len -= 3;
> -            out++;
> -        } else {
> -            *out++ = *in++;
> -            len--;
> -        }
> -    }
> -    *out = 0;
> -    return ret;
> +    return g_uri_unescape_string(lstr, NULL);
>  }
>
>  /**
> --
> 2.43.0
>
>

[-- Attachment #2: Type: text/html, Size: 4368 bytes --]

Re: [PATCH 2/5] util/uri: Simplify uri_string_unescape()

[Thomas Huth]

On 23/01/2024 11.25, Paolo Bonzini wrote:
> 
> 
> Il lun 22 gen 2024, 20:18 Thomas Huth <thuth@redhat.com 
> <mailto:thuth@redhat.com>> ha scritto:
> 
>     uri_string_unescape() basically does the same as the glib function
>     g_uri_unescape_string(), with just an additional length parameter.
> 
> 
> You can replace it altogether with g_uri_unescape_segment.

Oh, nice, I indeed missed that while looking at the glib docs! Thanks, I'll 
give it a try...

  Thomas