Re: [Qemu-devel] [PATCH 32/38] char: use a static array for backends

[Paolo Bonzini <pbonzini@redhat.com>]

On 22/10/2016 12:09, Marc-André Lureau wrote:
> Number and kinds of backends is known at compile-time, use a fixed-sized
> static array to simplify iterations & lookups.
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>  backends/baum.c       |   2 +-
>  backends/msmouse.c    |   2 +-
>  backends/testdev.c    |   2 +-
>  qemu-char.c           | 117 +++++++++++++++++++++++++++-----------------------
>  spice-qemu-char.c     |   4 +-
>  ui/console.c          |   2 +-
>  include/sysemu/char.h |   2 +-
>  7 files changed, 70 insertions(+), 61 deletions(-)
> 
> diff --git a/backends/baum.c b/backends/baum.c
> index 4fe11de..0a65c99 100644
> --- a/backends/baum.c
> +++ b/backends/baum.c
> @@ -644,7 +644,7 @@ fail_handle:
>  static void register_types(void)
>  {
>      static const CharDriver driver = {
> -        "braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL, chr_baum_init
> +        { "braille" }, CHARDEV_BACKEND_KIND_BRAILLE, NULL, chr_baum_init
>      };
>  
>      register_char_driver(&driver);
> diff --git a/backends/msmouse.c b/backends/msmouse.c
> index d6ab4ca..3367d67 100644
> --- a/backends/msmouse.c
> +++ b/backends/msmouse.c
> @@ -180,7 +180,7 @@ static CharDriverState *qemu_chr_open_msmouse(const char *id,
>  static void register_types(void)
>  {
>      static const CharDriver driver = {
> -        "msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL, qemu_chr_open_msmouse
> +        { "msmouse" }, CHARDEV_BACKEND_KIND_MSMOUSE, NULL, qemu_chr_open_msmouse
>      };
>      register_char_driver(&driver);
>  }
> diff --git a/backends/testdev.c b/backends/testdev.c
> index 5936189..d41352a 100644
> --- a/backends/testdev.c
> +++ b/backends/testdev.c
> @@ -131,7 +131,7 @@ static CharDriverState *chr_testdev_init(const char *id,
>  static void register_types(void)
>  {
>      static const CharDriver driver = {
> -        "testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL, chr_testdev_init
> +        { "testdev" }, CHARDEV_BACKEND_KIND_TESTDEV, NULL, chr_testdev_init
>      };
>      register_char_driver(&driver);
>  }
> diff --git a/qemu-char.c b/qemu-char.c
> index 594e795..7348cb0 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4042,20 +4042,20 @@ static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
>      }
>  }
>  
> -static GSList *backends;
> +static const CharDriver *backends[CHARDEV_BACKEND_KIND__MAX];
>  
>  void register_char_driver(const CharDriver *driver)
>  {
> -    backends = g_slist_append(backends, (void *)driver);
> +    backends[driver->kind] = driver;
>  }
>  
>  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>                                          Error **errp)
>  {
>      Error *local_err = NULL;
> -    CharDriver *cd;
> +    const CharDriver *cd;
>      CharDriverState *chr;
> -    GSList *i;
> +    int i;
>      ChardevReturn *ret = NULL;
>      ChardevBackend *backend;
>      const char *id = qemu_opts_id(opts);
> @@ -4069,9 +4069,14 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>  
>      if (is_help_option(qemu_opt_get(opts, "backend"))) {
>          fprintf(stderr, "Available chardev backend types:\n");
> -        for (i = backends; i; i = i->next) {
> -            cd = i->data;
> -            fprintf(stderr, "%s\n", cd->name);
> +        for (i = 0; i < ARRAY_SIZE(backends); i++) {
> +            cd = backends[i];
> +            if (cd) {
> +                fprintf(stderr, "%s\n", cd->name[0]);
> +                if (cd->name[1]) {
> +                    fprintf(stderr, "%s\n", cd->name[1]);
> +                }
> +            }
>          }
>          exit(!is_help_option(qemu_opt_get(opts, "backend")));
>      }
> @@ -4081,14 +4086,17 @@ CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
>          goto err;
>      }
>  
> -    for (i = backends; i; i = i->next) {
> -        cd = i->data;
> +    cd = NULL;
> +    for (i = 0; i < ARRAY_SIZE(backends); i++) {
> +        const char *name = qemu_opt_get(opts, "backend");
> +        cd = backends[i];
>  
> -        if (strcmp(cd->name, qemu_opt_get(opts, "backend")) == 0) {
> +        if (cd && (g_str_equal(cd->name[0], name) ||
> +                   (cd->name[1] && g_str_equal(cd->name[1], name)))) {
>              break;
>          }
>      }
> -    if (i == NULL) {
> +    if (cd == NULL) {
>          error_setg(errp, "chardev: backend \"%s\" not found",
>                     qemu_opt_get(opts, "backend"));
>          goto err;
> @@ -4293,20 +4301,32 @@ ChardevInfoList *qmp_query_chardev(Error **errp)
>      return chr_list;
>  }
>  
> +static ChardevBackendInfoList *
> +qmp_prepend_backend(ChardevBackendInfoList *list, const CharDriver *c,
> +                    const char *name)
> +{
> +    ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
> +    info->value = g_malloc0(sizeof(*info->value));
> +    info->value->name = g_strdup(name);
> +    info->next = list;
> +    return info;
> +
> +}
> +
>  ChardevBackendInfoList *qmp_query_chardev_backends(Error **errp)
>  {
>      ChardevBackendInfoList *backend_list = NULL;
> -    CharDriver *c = NULL;
> -    GSList *i = NULL;
> -
> -    for (i = backends; i; i = i->next) {
> -        ChardevBackendInfoList *info = g_malloc0(sizeof(*info));
> -        c = i->data;
> -        info->value = g_malloc0(sizeof(*info->value));
> -        info->value->name = g_strdup(c->name);
> +    const CharDriver *c;
> +    int i;
>  
> -        info->next = backend_list;
> -        backend_list = info;
> +    for (i = 0; i < ARRAY_SIZE(backends); i++) {
> +        c = backends[i];
> +        if (c) {
> +            backend_list = qmp_prepend_backend(backend_list, c, c->name[0]);
> +            if (c->name[1]) {
> +                backend_list = qmp_prepend_backend(backend_list, c, c->name[1]);
> +            }
> +        }
>      }
>  
>      return backend_list;
> @@ -4746,9 +4766,8 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>  {
>      ChardevReturn *ret = g_new0(ChardevReturn, 1);
>      CharDriverState *chr = NULL;
> +    const CharDriver *cd;
>      Error *local_err = NULL;
> -    GSList *i;
> -    CharDriver *cd;
>      bool be_opened = true;
>  
>      chr = qemu_chr_find(id);
> @@ -4757,22 +4776,16 @@ ChardevReturn *qmp_chardev_add(const char *id, ChardevBackend *backend,
>          goto out_error;
>      }
>  
> -    for (i = backends; i; i = i->next) {
> -        cd = i->data;
> -
> -        if (cd->kind == backend->type) {
> -            chr = cd->create(id, backend, ret, &be_opened, &local_err);
> -            if (local_err) {
> -                error_propagate(errp, local_err);
> -                goto out_error;
> -            }
> -            break;
> -        }
> +    cd = (int)backend->type >= 0 && backend->type < ARRAY_SIZE(backends) ?
> +        backends[backend->type] : NULL;
> +    if (cd == NULL) {
> +        error_setg(errp, "chardev backend not available");
> +        goto out_error;
>      }
>  
> -    if (chr == NULL) {
> -        assert(!i);
> -        error_setg(errp, "chardev backend not available");
> +    chr = cd->create(id, backend, ret, &be_opened, &local_err);
> +    if (local_err) {
> +        error_propagate(errp, local_err);
>          goto out_error;
>      }
>  
> @@ -4825,42 +4838,38 @@ static void register_types(void)
>  {
>      int i;
>      static const CharDriver drivers[] = {
> -        { "null", CHARDEV_BACKEND_KIND_NULL, NULL, qemu_chr_open_null },
> -        { "socket", CHARDEV_BACKEND_KIND_SOCKET,
> +        { { "null" }, CHARDEV_BACKEND_KIND_NULL, NULL, qemu_chr_open_null },
> +        { { "socket" }, CHARDEV_BACKEND_KIND_SOCKET,
>            qemu_chr_parse_socket, qmp_chardev_open_socket },
> -        { "udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
> +        { { "udp" }, CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
>            qmp_chardev_open_udp },
> -        { "ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> +        { { "ringbuf" }, CHARDEV_BACKEND_KIND_RINGBUF,
>            qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf },
> -        { "file", CHARDEV_BACKEND_KIND_FILE,
> +        { { "file" }, CHARDEV_BACKEND_KIND_FILE,
>            qemu_chr_parse_file_out, qmp_chardev_open_file },
> -        { "stdio", CHARDEV_BACKEND_KIND_STDIO,
> +        { { "stdio" }, CHARDEV_BACKEND_KIND_STDIO,
>            qemu_chr_parse_stdio, qemu_chr_open_stdio },
>  #if defined HAVE_CHARDEV_SERIAL
> -        { "serial", CHARDEV_BACKEND_KIND_SERIAL,
> -          qemu_chr_parse_serial, qmp_chardev_open_serial },
> -        { "tty", CHARDEV_BACKEND_KIND_SERIAL,
> +        { { "serial", "tty" }, CHARDEV_BACKEND_KIND_SERIAL,
>            qemu_chr_parse_serial, qmp_chardev_open_serial },
>  #endif
>  #ifdef HAVE_CHARDEV_PARPORT
> -        { "parallel", CHARDEV_BACKEND_KIND_PARALLEL,
> -          qemu_chr_parse_parallel, qmp_chardev_open_parallel },
> -        { "parport", CHARDEV_BACKEND_KIND_PARALLEL,
> +        { { "parallel", "parport" }, CHARDEV_BACKEND_KIND_PARALLEL,
>            qemu_chr_parse_parallel, qmp_chardev_open_parallel },
>  #endif
>  #ifdef HAVE_CHARDEV_PTY
> -        { "pty", CHARDEV_BACKEND_KIND_PTY, NULL, qemu_chr_open_pty },
> +        { { "pty" }, CHARDEV_BACKEND_KIND_PTY, NULL, qemu_chr_open_pty },
>  #endif
>  #ifdef _WIN32
> -        { "console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
> +        { { "console" }, CHARDEV_BACKEND_KIND_CONSOLE, NULL,
>            qemu_chr_open_win_con },
>  #endif
> -        { "pipe", CHARDEV_BACKEND_KIND_PIPE,
> +        { { "pipe" }, CHARDEV_BACKEND_KIND_PIPE,
>            qemu_chr_parse_pipe, qemu_chr_open_pipe },
> -        { "mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
> +        { { "mux" }, CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
>            qemu_chr_open_mux },
>          /* Bug-compatibility: */
> -        { "memory", CHARDEV_BACKEND_KIND_MEMORY,
> +        { { "memory" }, CHARDEV_BACKEND_KIND_MEMORY,
>            qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf },
>      };
>  
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 7ce8527..3172461 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -390,11 +390,11 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, ChardevBackend *backend,
>  static void register_types(void)
>  {
>      static const CharDriver vmc_driver = {
> -        "spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
> +        { "spicevmc" }, CHARDEV_BACKEND_KIND_SPICEVMC,
>          qemu_chr_parse_spice_vmc, qemu_chr_open_spice_vmc
>      };
>      static const CharDriver port_driver = {
> -        "spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
> +        { "spiceport" }, CHARDEV_BACKEND_KIND_SPICEPORT,
>          qemu_chr_parse_spice_port, qemu_chr_open_spice_port
>      };
>      register_char_driver(&vmc_driver);
> diff --git a/ui/console.c b/ui/console.c
> index a65223a..4e31cf2 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -2183,7 +2183,7 @@ static const TypeInfo qemu_console_info = {
>  static void register_types(void)
>  {
>      static const CharDriver vc_driver = {
> -        "vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc, vc_init
> +        { "vc" }, CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc, vc_init
>      };
>  
>      type_register_static(&qemu_console_info);
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 6a4b3ef..ee5618b 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -474,7 +474,7 @@ void qemu_chr_set_feature(CharDriverState *chr,
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>  
>  typedef struct CharDriver {
> -    const char *name;
> +    const char *name[2]; /* name & opt alias */

With designated initializers it is nicer to write this as

    const char *name;
    const char *alias;

instead.

Paolo

>      ChardevBackendKind kind;
>      void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
>      CharDriverState *(*create)(const char *id,
>