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

Markus Armbruster posted 10 patches 8 years, 10 months ago
There is a newer version of this series
[Qemu-devel] [RFC v2 for-2.9 07/10] sockets: New helper socket_address_crumple()
Posted by Markus Armbruster 8 years, 10 months ago
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 <armbru@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
+ *
+ * Convert SocketAddressFlat to SocketAddress.  Caller is responsible
+ * for freeing with qapi_free_SocketAddress().
+ *
+ * Returns: the argument converted to SocketAddress.
+ */
+SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat);
+
 #endif /* QEMU_SOCKETS_H */
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 4ae37bd..03a2a47 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -25,6 +25,7 @@
 #include "qapi/error.h"
 #include "qemu/sockets.h"
 #include "qemu/main-loop.h"
+#include "qapi/clone-visitor.h"
 #include "qapi/qobject-input-visitor.h"
 #include "qapi/qobject-output-visitor.h"
 #include "qapi-visit.h"
@@ -1341,3 +1342,31 @@ char *socket_address_to_string(struct SocketAddress *addr, Error **errp)
     }
     return buf;
 }
+
+SocketAddress *socket_address_crumple(SocketAddressFlat *addr_flat)
+{
+    SocketAddress *addr = g_new(SocketAddress, 1);
+
+    addr->type = addr_flat->type;
+    switch (addr->type) {
+    case SOCKET_ADDRESS_FLAT_TYPE_INET:
+        addr->u.inet.data = QAPI_CLONE(InetSocketAddress,
+                                        &addr_flat->u.inet);
+        break;
+    case SOCKET_ADDRESS_FLAT_TYPE_UNIX:
+        addr->u.q_unix.data = QAPI_CLONE(UnixSocketAddress, &
+                                          addr_flat->u.q_unix);
+        break;
+    case SOCKET_ADDRESS_FLAT_TYPE_VSOCK:
+        addr->u.vsock.data = QAPI_CLONE(VsockSocketAddress,
+                                         &addr_flat->u.vsock);
+        break;
+    case SOCKET_ADDRESS_FLAT_TYPE_FD:
+        addr->u.fd.data = QAPI_CLONE(String, &addr_flat->u.fd);
+        break;
+    default:
+        abort();
+    }
+
+    return addr;
+}
-- 
2.7.4


Re: [Qemu-devel] [RFC v2 for-2.9 07/10] sockets: New helper socket_address_crumple()
Posted by Eric Blake 8 years, 10 months ago
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 <armbru@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/

> +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