Re: [Qemu-devel] [PATCH v2 1/9] char: move QemuOpts->ChardevBackend translation to a separate func

[Marc-André Lureau]

Hi

On Fri, May 19, 2017 at 4:51 PM Anton Nefedov <address@hidden>
wrote:

> Signed-off-by: Anton Nefedov <address@hidden>
> Reviewed-by: Vladimir Sementsov-Ogievskiy <address@hidden>
> ---
>  chardev/char.c | 72
> +++++++++++++++++++++++++++++++++++++---------------------
>  1 file changed, 46 insertions(+), 26 deletions(-)
>

Motivation is the upcoming patch for hmp_chardev_change()


>
> diff --git a/chardev/char.c b/chardev/char.c
> index 4e24dc3..684cccd 100644
> --- a/chardev/char.c
> +++ b/chardev/char.c
> @@ -854,17 +854,14 @@ help_string_append(const char *name, void *opaque)
>      g_string_append_printf(str, "\n%s", name);
>  }
>
> -Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
> -                                Error **errp)
> +static ChardevBackend *qemu_chr_parse_opts(QemuOpts *opts,
> +                                           Error **errp)
>

while at it, make it a single line?


>  {
>      Error *local_err = NULL;
>      const ChardevClass *cc;
> -    Chardev *chr;
>      int i;
>      ChardevBackend *backend = NULL;
>      const char *name = qemu_opt_get(opts, "backend");
> -    const char *id = qemu_opts_id(opts);
> -    char *bid = NULL;
>
>      if (name == NULL) {
>          error_setg(errp, "chardev: \"%s\" missing backend",
> @@ -872,21 +869,6 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
>          return NULL;
>      }
>
> -    if (is_help_option(name)) {
> -        GString *str = g_string_new("");
> -
> -        chardev_name_foreach(help_string_append, str);
> -
> -        error_report("Available chardev backend types: %s", str->str);
> -        g_string_free(str, true);
> -        exit(0);
> -    }
> -
> -    if (id == NULL) {
> -        error_setg(errp, "chardev: no id specified");
> -        return NULL;
> -    }
> -
>      for (i = 0; i < ARRAY_SIZE(chardev_alias_table); i++) {
>          if (g_strcmp0(chardev_alias_table[i].alias, name) == 0) {
>              name = chardev_alias_table[i].typename;
> @@ -902,16 +884,12 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
>      backend = g_new0(ChardevBackend, 1);
>      backend->type = CHARDEV_BACKEND_KIND_NULL;
>
> -    if (qemu_opt_get_bool(opts, "mux", 0)) {
> -        bid = g_strdup_printf("%s-base", id);
> -    }
> -
> -    chr = NULL;
>      if (cc->parse) {
>          cc->parse(opts, backend, &local_err);
>          if (local_err) {
>              error_propagate(errp, local_err);
> -            goto out;
> +            qapi_free_ChardevBackend(backend);
> +            return NULL;
>          }
>      } else {
>          ChardevCommon *ccom = g_new0(ChardevCommon, 1);
> @@ -919,6 +897,48 @@ Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
>          backend->u.null.data = ccom; /* Any ChardevCommon member would
> work */
>      }
>
> +    return backend;
> +}
> +
> +Chardev *qemu_chr_new_from_opts(QemuOpts *opts,
> +                                Error **errp)
>

same here


> +{
> +    const ChardevClass *cc;
> +    Chardev *chr = NULL;
> +    ChardevBackend *backend = NULL;
> +    const char *name = qemu_opt_get(opts, "backend");
> +    const char *id = qemu_opts_id(opts);
> +    char *bid = NULL;
> +
> +    if (name && is_help_option(name)) {
> +        GString *str = g_string_new("");
> +
> +        chardev_name_foreach(help_string_append, str);
> +
> +        error_report("Available chardev backend types: %s", str->str);
> +        g_string_free(str, true);
> +        exit(0);
> +    }
> +
> +    if (id == NULL) {
> +        error_setg(errp, "chardev: no id specified");
> +        return NULL;
> +    }
> +
> +    backend = qemu_chr_parse_opts(opts, errp);
> +    if (backend == NULL) {
> +        return NULL;
> +    }
> +
> +    cc = char_get_class(name, errp);
> +    if (cc == NULL) {
> +        goto out;
> +    }
> +
> +    if (qemu_opt_get_bool(opts, "mux", 0)) {
> +        bid = g_strdup_printf("%s-base", id);
> +    }
> +
>      chr = qemu_chardev_new(bid ? bid : id,
>                             object_class_get_name(OBJECT_CLASS(cc)),
>                             backend, errp);
> --
> 2.7.4
>
>
> --
Marc-André Lureau