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

["Dr. David Alan Gilbert" <dgilbert@redhat.com>]

* 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