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