Re: [Qemu-devel] [PATCH 31/38] char: use a const CharDriver

[Paolo Bonzini]

On 22/10/2016 12:09, Marc-André Lureau wrote:
> No need to allocate & copy fileds, let's use static const struct
> instead.
> 
> Signed-off-by: Marc-André Lureau <address@hidden>
> ---
>  backends/baum.c       |  7 ++--
>  backends/msmouse.c    |  6 ++--
>  backends/testdev.c    |  6 ++--
>  qemu-char.c           | 89 
> +++++++++++++++++++++++----------------------------
>  spice-qemu-char.c     | 14 +++++---
>  ui/console.c          |  8 +++--
>  include/sysemu/char.h | 20 ++++++------
>  7 files changed, 79 insertions(+), 71 deletions(-)
> 
> diff --git a/backends/baum.c b/backends/baum.c
> index 919844e..4fe11de 100644
> --- a/backends/baum.c
> +++ b/backends/baum.c
> @@ -643,8 +643,11 @@ fail_handle:
>  
>  static void register_types(void)
>  {
> -    register_char_driver("braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL,
> -                         chr_baum_init);
> +    static const CharDriver driver = {
> +        "braille", CHARDEV_BACKEND_KIND_BRAILLE, NULL, chr_baum_init

As mentioned in the review to patch 32, please use designated
initializers for this.

But actually const char *name is superfluous even, because you can use
ChardevBackendKind_lookup[cd->kind] instead.

Paolo

> +    };
> +
> +    register_char_driver(&driver);
>  }
>  
>  type_init(register_types);
> diff --git a/backends/msmouse.c b/backends/msmouse.c
> index 733ca80..d6ab4ca 100644
> --- a/backends/msmouse.c
> +++ b/backends/msmouse.c
> @@ -179,8 +179,10 @@ static CharDriverState *qemu_chr_open_msmouse(const char 
> *id,
>  
>  static void register_types(void)
>  {
> -    register_char_driver("msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL,
> -                         qemu_chr_open_msmouse);
> +    static const CharDriver driver = {
> +        "msmouse", CHARDEV_BACKEND_KIND_MSMOUSE, NULL, qemu_chr_open_msmouse
> +    };
> +    register_char_driver(&driver);
>  }
>  
>  type_init(register_types);
> diff --git a/backends/testdev.c b/backends/testdev.c
> index 60156e3..5936189 100644
> --- a/backends/testdev.c
> +++ b/backends/testdev.c
> @@ -130,8 +130,10 @@ static CharDriverState *chr_testdev_init(const char *id,
>  
>  static void register_types(void)
>  {
> -    register_char_driver("testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL,
> -                         chr_testdev_init);
> +    static const CharDriver driver = {
> +        "testdev", CHARDEV_BACKEND_KIND_TESTDEV, NULL, chr_testdev_init
> +    };
> +    register_char_driver(&driver);
>  }
>  
>  type_init(register_types);
> diff --git a/qemu-char.c b/qemu-char.c
> index 8e05c8c..594e795 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -4042,27 +4042,11 @@ static void qemu_chr_parse_udp(QemuOpts *opts, 
> ChardevBackend *backend,
>      }
>  }
>  
> -typedef struct CharDriver {
> -    const char *name;
> -    ChardevBackendKind kind;
> -    CharDriverParse *parse;
> -    CharDriverCreate *create;
> -} CharDriver;
> -
>  static GSList *backends;
>  
> -void register_char_driver(const char *name, ChardevBackendKind kind,
> -                          CharDriverParse *parse, CharDriverCreate *create)
> +void register_char_driver(const CharDriver *driver)
>  {
> -    CharDriver *s;
> -
> -    s = g_malloc0(sizeof(*s));
> -    s->name = g_strdup(name);
> -    s->kind = kind;
> -    s->parse = parse;
> -    s->create = create;
> -
> -    backends = g_slist_append(backends, s);
> +    backends = g_slist_append(backends, (void *)driver);
>  }
>  
>  CharDriverState *qemu_chr_new_from_opts(QemuOpts *opts,
> @@ -4839,45 +4823,52 @@ void qemu_chr_cleanup(void)
>  
>  static void register_types(void)
>  {
> -    register_char_driver("null", CHARDEV_BACKEND_KIND_NULL, NULL,
> -                         qemu_chr_open_null);
> -    register_char_driver("socket", CHARDEV_BACKEND_KIND_SOCKET,
> -                         qemu_chr_parse_socket, qmp_chardev_open_socket);
> -    register_char_driver("udp", CHARDEV_BACKEND_KIND_UDP, qemu_chr_parse_udp,
> -                         qmp_chardev_open_udp);
> -    register_char_driver("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> -                         qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf);
> -    register_char_driver("file", CHARDEV_BACKEND_KIND_FILE,
> -                         qemu_chr_parse_file_out, qmp_chardev_open_file);
> -    register_char_driver("stdio", CHARDEV_BACKEND_KIND_STDIO,
> -                         qemu_chr_parse_stdio, qemu_chr_open_stdio);
> +    int i;
> +    static const CharDriver drivers[] = {
> +        { "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,
> +          qmp_chardev_open_udp },
> +        { "ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
> +          qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf },
> +        { "file", CHARDEV_BACKEND_KIND_FILE,
> +          qemu_chr_parse_file_out, qmp_chardev_open_file },
> +        { "stdio", CHARDEV_BACKEND_KIND_STDIO,
> +          qemu_chr_parse_stdio, qemu_chr_open_stdio },
>  #if defined HAVE_CHARDEV_SERIAL
> -    register_char_driver("serial", CHARDEV_BACKEND_KIND_SERIAL,
> -                         qemu_chr_parse_serial, qmp_chardev_open_serial);
> -    register_char_driver("tty", CHARDEV_BACKEND_KIND_SERIAL,
> -                         qemu_chr_parse_serial, qmp_chardev_open_serial);
> +        { "serial", CHARDEV_BACKEND_KIND_SERIAL,
> +          qemu_chr_parse_serial, qmp_chardev_open_serial },
> +        { "tty", CHARDEV_BACKEND_KIND_SERIAL,
> +          qemu_chr_parse_serial, qmp_chardev_open_serial },
>  #endif
>  #ifdef HAVE_CHARDEV_PARPORT
> -    register_char_driver("parallel", CHARDEV_BACKEND_KIND_PARALLEL,
> -                         qemu_chr_parse_parallel, qmp_chardev_open_parallel);
> -    register_char_driver("parport", CHARDEV_BACKEND_KIND_PARALLEL,
> -                         qemu_chr_parse_parallel, qmp_chardev_open_parallel);
> +        { "parallel", CHARDEV_BACKEND_KIND_PARALLEL,
> +          qemu_chr_parse_parallel, qmp_chardev_open_parallel },
> +        { "parport", CHARDEV_BACKEND_KIND_PARALLEL,
> +          qemu_chr_parse_parallel, qmp_chardev_open_parallel },
>  #endif
>  #ifdef HAVE_CHARDEV_PTY
> -    register_char_driver("pty", CHARDEV_BACKEND_KIND_PTY, NULL,
> -                         qemu_chr_open_pty);
> +        { "pty", CHARDEV_BACKEND_KIND_PTY, NULL, qemu_chr_open_pty },
>  #endif
>  #ifdef _WIN32
> -    register_char_driver("console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
> -                         qemu_chr_open_win_con);
> +        { "console", CHARDEV_BACKEND_KIND_CONSOLE, NULL,
> +          qemu_chr_open_win_con },
>  #endif
> -    register_char_driver("pipe", CHARDEV_BACKEND_KIND_PIPE,
> -                         qemu_chr_parse_pipe, qemu_chr_open_pipe);
> -    register_char_driver("mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
> -                         qemu_chr_open_mux);
> -    /* Bug-compatibility: */
> -    register_char_driver("memory", CHARDEV_BACKEND_KIND_MEMORY,
> -                         qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf);
> +        { "pipe", CHARDEV_BACKEND_KIND_PIPE,
> +          qemu_chr_parse_pipe, qemu_chr_open_pipe },
> +        { "mux", CHARDEV_BACKEND_KIND_MUX, qemu_chr_parse_mux,
> +          qemu_chr_open_mux },
> +        /* Bug-compatibility: */
> +        { "memory", CHARDEV_BACKEND_KIND_MEMORY,
> +          qemu_chr_parse_ringbuf, qemu_chr_open_ringbuf },
> +    };
> +
> +
> +    for (i = 0; i < ARRAY_SIZE(drivers); i++) {
> +        register_char_driver(&drivers[i]);
> +    }
> +
>      /* this must be done after machine init, since we register FEs with muxes
>       * as part of realize functions like serial_isa_realizefn when -nographic
>       * is specified
> diff --git a/spice-qemu-char.c b/spice-qemu-char.c
> index 276c4ae..7ce8527 100644
> --- a/spice-qemu-char.c
> +++ b/spice-qemu-char.c
> @@ -389,10 +389,16 @@ static void qemu_chr_parse_spice_port(QemuOpts *opts, 
> ChardevBackend *backend,
>  
>  static void register_types(void)
>  {
> -    register_char_driver("spicevmc", CHARDEV_BACKEND_KIND_SPICEVMC,
> -                         qemu_chr_parse_spice_vmc, qemu_chr_open_spice_vmc);
> -    register_char_driver("spiceport", CHARDEV_BACKEND_KIND_SPICEPORT,
> -                         qemu_chr_parse_spice_port, 
> qemu_chr_open_spice_port);
> +    static const CharDriver vmc_driver = {
> +        "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,
> +        qemu_chr_parse_spice_port, qemu_chr_open_spice_port
> +    };
> +    register_char_driver(&vmc_driver);
> +    register_char_driver(&port_driver);
>  }
>  
>  type_init(register_types);
> diff --git a/ui/console.c b/ui/console.c
> index ed888e5..a65223a 100644
> --- a/ui/console.c
> +++ b/ui/console.c
> @@ -2180,12 +2180,14 @@ static const TypeInfo qemu_console_info = {
>      .class_size = sizeof(QemuConsoleClass),
>  };
>  
> -
>  static void register_types(void)
>  {
> +    static const CharDriver vc_driver = {
> +        "vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc, vc_init
> +    };
> +
>      type_register_static(&qemu_console_info);
> -    register_char_driver("vc", CHARDEV_BACKEND_KIND_VC, qemu_chr_parse_vc,
> -                         vc_init);
> +    register_char_driver(&vc_driver);
>  }
>  
>  type_init(register_types);
> diff --git a/include/sysemu/char.h b/include/sysemu/char.h
> index 0a14942..6a4b3ef 100644
> --- a/include/sysemu/char.h
> +++ b/include/sysemu/char.h
> @@ -473,15 +473,17 @@ void qemu_chr_set_feature(CharDriverState *chr,
>                            CharDriverFeature feature);
>  QemuOpts *qemu_chr_parse_compat(const char *label, const char *filename);
>  
> -typedef void CharDriverParse(QemuOpts *opts, ChardevBackend *backend,
> -                             Error **errp);
> -typedef CharDriverState *CharDriverCreate(const char *id,
> -                                          ChardevBackend *backend,
> -                                          ChardevReturn *ret, bool 
> *be_opened,
> -                                          Error **errp);
> -
> -void register_char_driver(const char *name, ChardevBackendKind kind,
> -                          CharDriverParse *parse, CharDriverCreate *create);
> +typedef struct CharDriver {
> +    const char *name;
> +    ChardevBackendKind kind;
> +    void (*parse)(QemuOpts *opts, ChardevBackend *backend, Error **errp);
> +    CharDriverState *(*create)(const char *id,
> +                               ChardevBackend *backend,
> +                               ChardevReturn *ret, bool *be_opened,
> +                               Error **errp);
> +} CharDriver;
> +
> +void register_char_driver(const CharDriver *driver);
>  
>  extern int term_escape_char;
>  
>