Re: [Qemu-block] [Qemu-devel] [PATCH v2 05/19] util: Shorten references into SocketAddress

[Markus Armbruster]

Eric Blake <address@hidden> writes:

> An upcoming patch will alter how simple unions, like SocketAddress,
> are laid out, which will impact all lines of the form 'addr->u.XXX'.
> To minimize the impact of that patch, use C99 initialization or a
> temporary variable to reduce the number of lines needing modification
> when an internal reference within SocketAddress changes layout.
>
> Signed-off-by: Eric Blake <address@hidden>
> Reviewed-by: Daniel P. Berrange <address@hidden>

If you improve the previous commit's message to address my remarks,
you'll probably want to update this one, too.
>
> ---
> v2: add R-b
> ---
>  block/nbd.c                    | 14 ++++++++------
>  qemu-char.c                    | 43 
> ++++++++++++++++++++++++------------------
>  qemu-nbd.c                     |  9 +++++----
>  tests/test-io-channel-socket.c | 26 ++++++++++++++++---------
>  ui/vnc.c                       | 39 +++++++++++++++++++-------------------
>  util/qemu-sockets.c            | 11 ++++++-----
>  6 files changed, 81 insertions(+), 61 deletions(-)
>
> diff --git a/block/nbd.c b/block/nbd.c
> index db57b49..9f333c9 100644
> --- a/block/nbd.c
> +++ b/block/nbd.c
> @@ -204,18 +204,20 @@ static SocketAddress *nbd_config(BDRVNBDState *s, QDict 
> *options, char **export,
>      saddr = g_new0(SocketAddress, 1);
>
>      if (qdict_haskey(options, "path")) {
> +        UnixSocketAddress *q_unix;
>          saddr->type = SOCKET_ADDRESS_KIND_UNIX;
> -        saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
> -        saddr->u.q_unix->path = g_strdup(qdict_get_str(options, "path"));
> +        q_unix = saddr->u.q_unix = g_new0(UnixSocketAddress, 1);
> +        q_unix->path = g_strdup(qdict_get_str(options, "path"));
>          qdict_del(options, "path");
>      } else {
> +        InetSocketAddress *inet;
>          saddr->type = SOCKET_ADDRESS_KIND_INET;
> -        saddr->u.inet = g_new0(InetSocketAddress, 1);
> -        saddr->u.inet->host = g_strdup(qdict_get_str(options, "host"));
> +        inet = saddr->u.inet = g_new0(InetSocketAddress, 1);
> +        inet->host = g_strdup(qdict_get_str(options, "host"));
>          if (!qdict_get_try_str(options, "port")) {
> -            saddr->u.inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
> +            inet->port = g_strdup_printf("%d", NBD_DEFAULT_PORT);
>          } else {
> -            saddr->u.inet->port = g_strdup(qdict_get_str(options, "port"));
> +            inet->port = g_strdup(qdict_get_str(options, "port"));
>          }
>          qdict_del(options, "host");
>          qdict_del(options, "port");
> diff --git a/qemu-char.c b/qemu-char.c
> index 5ea1d34..cfc82bc 100644
> --- a/qemu-char.c
> +++ b/qemu-char.c
> @@ -3659,20 +3659,23 @@ static void qemu_chr_parse_socket(QemuOpts *opts, 
> ChardevBackend *backend,
>
>      addr = g_new0(SocketAddress, 1);
>      if (path) {
> +        UnixSocketAddress *q_unix;
>          addr->type = SOCKET_ADDRESS_KIND_UNIX;
> -        addr->u.q_unix = g_new0(UnixSocketAddress, 1);
> -        addr->u.q_unix->path = g_strdup(path);
> +        q_unix = addr->u.q_unix = g_new0(UnixSocketAddress, 1);
> +        q_unix->path = g_strdup(path);
>      } else {
>          addr->type = SOCKET_ADDRESS_KIND_INET;
>          addr->u.inet = g_new0(InetSocketAddress, 1);
> -        addr->u.inet->host = g_strdup(host);
> -        addr->u.inet->port = g_strdup(port);
> -        addr->u.inet->has_to = qemu_opt_get(opts, "to");
> -        addr->u.inet->to = qemu_opt_get_number(opts, "to", 0);
> -        addr->u.inet->has_ipv4 = qemu_opt_get(opts, "ipv4");
> -        addr->u.inet->ipv4 = qemu_opt_get_bool(opts, "ipv4", 0);
> -        addr->u.inet->has_ipv6 = qemu_opt_get(opts, "ipv6");
> -        addr->u.inet->ipv6 = qemu_opt_get_bool(opts, "ipv6", 0);
> +        *addr->u.inet = (InetSocketAddress) {
> +            .host = g_strdup(host),
> +            .port = g_strdup(port),
> +            .has_to = qemu_opt_get(opts, "to"),
> +            .to = qemu_opt_get_number(opts, "to", 0),
> +            .has_ipv4 = qemu_opt_get(opts, "ipv4"),
> +            .ipv4 = qemu_opt_get_bool(opts, "ipv4", 0),
> +            .has_ipv6 = qemu_opt_get(opts, "ipv6"),
> +            .ipv6 = qemu_opt_get_bool(opts, "ipv6", 0),
> +        };

Do you still need g_new0(), or would g_new() do?

>      }
>      sock->addr = addr;
>  }
[More of the same snipped...]