On 30.03.2017 17:03, Markus Armbruster wrote:
> Eric Blake <ebl...@redhat.com> writes:
>
>> 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 <arm...@redhat.com>
>>> ---
>>> 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/
>
> Fixing...
>
>>> +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.
>
> Sold.
>
>>> + 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?
>
> Editing accident.
>
>>> + 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;
>>> +}
>>>
>
> Now looks like this:
>
> SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat)
> {
> SocketAddress *addr = g_new(SocketAddress, 1);
>
> switch (addr_flat->type) {
> case SOCKET_ADDRESS_FLAT_TYPE_INET:
> addr->type = SOCKET_ADDRESS_KIND_INET;
> addr->u.inet.data = QAPI_CLONE(InetSocketAddress,
> &addr_flat->u.inet);
> break;
> case SOCKET_ADDRESS_FLAT_TYPE_UNIX:
> addr->type = SOCKET_ADDRESS_KIND_UNIX;
> addr->u.q_unix.data = QAPI_CLONE(UnixSocketAddress,
> &addr_flat->u.q_unix);
> break;
> case SOCKET_ADDRESS_FLAT_TYPE_VSOCK:
> addr->type = SOCKET_ADDRESS_KIND_VSOCK;
> addr->u.vsock.data = QAPI_CLONE(VsockSocketAddress,
> &addr_flat->u.vsock);
> break;
> case SOCKET_ADDRESS_FLAT_TYPE_FD:
> addr->type = SOCKET_ADDRESS_KIND_FD;
> addr->u.fd.data = QAPI_CLONE(String, &addr_flat->u.fd);
> break;
> default:
> abort();
> }
>
> return addr;
> }Looks good! Max
signature.asc
Description: OpenPGP digital signature
