Eric Blake <ebl...@redhat.com> 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 <ebl...@redhat.com>
> Reviewed-by: Daniel P. Berrange <berra...@redhat.com>
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...]
