[PATCH 11/13] hmp/char: Use qemu_chr_parse_cli_str() for chardev-change

Kevin Wolf posted 13 patches 4 years, 10 months ago
Maintainers: "Dr. David Alan Gilbert" <dgilbert@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Eric Blake <eblake@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Gerd Hoffmann <kraxel@redhat.com>, Markus Armbruster <armbru@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>
[PATCH 11/13] hmp/char: Use qemu_chr_parse_cli_str() for chardev-change
Posted by Kevin Wolf 4 years, 10 months ago
Instead of going through the QemuOpts-based parser, go directly from the
given option string to ChardevOptions. This doesn't only avoid legacy
code, but it also simplifies the implementation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 monitor/hmp-cmds.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index a6a6684df1..0244068de8 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -1793,34 +1793,25 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict)
 void hmp_chardev_change(Monitor *mon, const QDict *qdict)
 {
     const char *args = qdict_get_str(qdict, "args");
-    const char *id;
+    const char *id = qdict_get_str(qdict, "id");
+    char *optstr;
     Error *err = NULL;
-    ChardevBackend *backend = NULL;
+    ChardevOptions *options = NULL;
     ChardevReturn *ret = NULL;
-    QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), args,
-                                             true);
-    if (!opts) {
-        error_setg(&err, "Parsing chardev args failed");
-        goto end;
-    }
 
-    id = qdict_get_str(qdict, "id");
-    if (qemu_opts_id(opts)) {
-        error_setg(&err, "Unexpected 'id' parameter");
-        goto end;
-    }
+    optstr = g_strdup_printf("%s,id=%s", args, id);
 
-    backend = qemu_chr_parse_opts(opts, &err);
-    if (!backend) {
+    options = qemu_chr_parse_cli_str(optstr, &err);
+    if (!options) {
         goto end;
     }
 
-    ret = qmp_chardev_change(id, backend, &err);
+    ret = qmp_chardev_change(options->id, options->backend, &err);
 
 end:
+    g_free(optstr);
     qapi_free_ChardevReturn(ret);
-    qapi_free_ChardevBackend(backend);
-    qemu_opts_del(opts);
+    qapi_free_ChardevOptions(options);
     hmp_handle_error(mon, err);
 }
 
-- 
2.28.0


Re: [PATCH 11/13] hmp/char: Use qemu_chr_parse_cli_str() for chardev-change
Posted by Dr. David Alan Gilbert 4 years, 10 months ago
* Kevin Wolf (kwolf@redhat.com) wrote:
> Instead of going through the QemuOpts-based parser, go directly from the
> given option string to ChardevOptions. This doesn't only avoid legacy
> code, but it also simplifies the implementation.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>

OK, from HMP I think

Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>

I'm assuming there's no change in the escaping from you extracting it
from the qdict and then printfing it back to go throguh the parser?

Dave

> ---
>  monitor/hmp-cmds.c | 27 +++++++++------------------
>  1 file changed, 9 insertions(+), 18 deletions(-)
> 
> diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
> index a6a6684df1..0244068de8 100644
> --- a/monitor/hmp-cmds.c
> +++ b/monitor/hmp-cmds.c
> @@ -1793,34 +1793,25 @@ void hmp_chardev_add(Monitor *mon, const QDict *qdict)
>  void hmp_chardev_change(Monitor *mon, const QDict *qdict)
>  {
>      const char *args = qdict_get_str(qdict, "args");
> -    const char *id;
> +    const char *id = qdict_get_str(qdict, "id");
> +    char *optstr;
>      Error *err = NULL;
> -    ChardevBackend *backend = NULL;
> +    ChardevOptions *options = NULL;
>      ChardevReturn *ret = NULL;
> -    QemuOpts *opts = qemu_opts_parse_noisily(qemu_find_opts("chardev"), args,
> -                                             true);
> -    if (!opts) {
> -        error_setg(&err, "Parsing chardev args failed");
> -        goto end;
> -    }
>  
> -    id = qdict_get_str(qdict, "id");
> -    if (qemu_opts_id(opts)) {
> -        error_setg(&err, "Unexpected 'id' parameter");
> -        goto end;
> -    }
> +    optstr = g_strdup_printf("%s,id=%s", args, id);
>  
> -    backend = qemu_chr_parse_opts(opts, &err);
> -    if (!backend) {
> +    options = qemu_chr_parse_cli_str(optstr, &err);
> +    if (!options) {
>          goto end;
>      }
>  
> -    ret = qmp_chardev_change(id, backend, &err);
> +    ret = qmp_chardev_change(options->id, options->backend, &err);
>  
>  end:
> +    g_free(optstr);
>      qapi_free_ChardevReturn(ret);
> -    qapi_free_ChardevBackend(backend);
> -    qemu_opts_del(opts);
> +    qapi_free_ChardevOptions(options);
>      hmp_handle_error(mon, err);
>  }
>  
> -- 
> 2.28.0
> 
-- 
Dr. David Alan Gilbert / dgilbert@redhat.com / Manchester, UK