Re: [Qemu-block] [RFC v2 for-2.9 07/10] sockets: New helper socket_address_crumple()

[Eric Blake]

On 03/30/2017 08:15 AM, Markus Armbruster wrote:
> SocketAddress is a simple union, and simple unions are awkward: they
> have their variant members wrapped in a "data" object on the wire, and
> require additional indirections in C.  I intend to limit its use to
> existing external interfaces.  New ones should use SocketAddressFlat.
> I further intend to convert all internal interfaces to
> SocketAddressFlat.  This helper should go away then.
> 
> Signed-off-by: Markus Armbruster <address@hidden>
> ---
>  include/qemu/sockets.h | 11 +++++++++++
>  util/qemu-sockets.c    | 29 +++++++++++++++++++++++++++++
>  2 files changed, 40 insertions(+)
> 
> diff --git a/include/qemu/sockets.h b/include/qemu/sockets.h
> index 5f1bab9..cef05a5 100644
> --- a/include/qemu/sockets.h
> +++ b/include/qemu/sockets.h
> @@ -119,4 +119,15 @@ SocketAddress *socket_remote_address(int fd, Error 
> **errp);
>   */
>  char *socket_address_to_string(struct SocketAddress *addr, Error **errp);
>  
> +/**
> + * socket_address_crumple:
> + * @addr_flat: the socket addres to crumple

s/addres/address/

> +SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat)
> +{
> +    SocketAddress *addr = g_new(SocketAddress, 1);
> +
> +    addr->type = addr_flat->type;

Works only because our enum is defined in the same order as the simple
union's members.  A bit fragile, so maybe we want to comment in the
.json file that we can't reorder members of either the enum or the
simple union's 'data'?  Or it might even tickle a picky compiler to warn
about assignment between incompatible enum types.  Another option would
be making it robust by instead doing switch(addr_flat->type) and
assigning to addr->type in each branch of the switch.

> +    switch (addr->type) {
> +    case SOCKET_ADDRESS_FLAT_TYPE_INET:
> +        addr->u.inet.data = QAPI_CLONE(InetSocketAddress,
> +                                        &addr_flat->u.inet);

Indentation is off.

> +        break;
> +    case SOCKET_ADDRESS_FLAT_TYPE_UNIX:
> +        addr->u.q_unix.data = QAPI_CLONE(UnixSocketAddress, &
> +                                          addr_flat->u.q_unix);

Why is the unary & split from its argument?

> +        break;
> +    case SOCKET_ADDRESS_FLAT_TYPE_VSOCK:
> +        addr->u.vsock.data = QAPI_CLONE(VsockSocketAddress,
> +                                         &addr_flat->u.vsock);

More indentation problems.

> +        break;
> +    case SOCKET_ADDRESS_FLAT_TYPE_FD:
> +        addr->u.fd.data = QAPI_CLONE(String, &addr_flat->u.fd);
> +        break;
> +    default:
> +        abort();
> +    }
> +
> +    return addr;
> +}
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature