Re: [PATCH 18/20] keyval: Use GString to accumulate value strings

[Paolo Bonzini]

On 11/12/20 18:11, Markus Armbruster wrote:
> QString supports modifying its string, but it's quite limited: you can
> only append.  The remaining callers use it for building an initial
> string, never for modifying it later.
>
> Change keyval_parse_one() to do build the initial string with GString.
> This is another step towards making QString immutable.
>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>

It's a bit unfortunate that the infamous "keyval: accept escaped commas 
in implied option" patch was already getting rid of mutable QString.

It used a completely different mechanism, namely unescaping the string 
in place.  This means that my patch was doing n+1 allocations, versus a 
best case of n and a generic case of O(n) for this patch.  The 
difference does not really matter, though I still like my code better.

Paolo

> ---
>   util/keyval.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/util/keyval.c b/util/keyval.c
> index 7f625ad33c..be34928813 100644
> --- a/util/keyval.c
> +++ b/util/keyval.c
> @@ -189,7 +189,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
> char *params,
>       QDict *cur;
>       int ret;
>       QObject *next;
> -    QString *val;
> +    GString *val;
>   
>       key = params;
>       val_end = NULL;
> @@ -263,7 +263,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
> char *params,
>   
>       if (key == implied_key) {
>           assert(!*s);
> -        val = qstring_from_substr(params, 0, val_end - params);
> +        val = g_string_new_len(params, val_end - params);
>           s = val_end;
>           if (*s == ',') {
>               s++;
> @@ -276,7 +276,7 @@ static const char *keyval_parse_one(QDict *qdict, const 
> char *params,
>           }
>           s++;
>   
> -        val = qstring_new();
> +        val = g_string_new(NULL);
>           for (;;) {
>               if (!*s) {
>                   break;
> @@ -286,11 +286,12 @@ static const char *keyval_parse_one(QDict *qdict, const 
> char *params,
>                       break;
>                   }
>               }
> -            qstring_append_chr(val, *s++);
> +            g_string_append_c(val, *s++);
>           }
>       }
>   
> -    if (!keyval_parse_put(cur, key_in_cur, val, key, key_end, errp)) {
> +    if (!keyval_parse_put(cur, key_in_cur, qstring_from_gstring(val),
> +                          key, key_end, errp)) {
>           return NULL;
>       }
>       return s;