diff mbox

[12/16] convert net_init_socket() to NetClientOptions

Message ID 1337683555-13301-13-git-send-email-lersek@redhat.com
State New
Headers show

Commit Message

Laszlo Ersek May 22, 2012, 10:45 a.m. UTC 
I "reverse engineered" the following permissions between the -socket
sub-options:

             fd  listen  connect  mcast  udp | localaddr
  fd         x   .       .        .      .   | .
  listen     .   x       .        .      .   | .
  connect    .   .       x        .      .   | .
  mcast      .   .       .        x      .   | x
  udp        .   .       .        .      x   | x
  -------------------------------------------+
  localaddr  .   .       .        x      x     x

I transformed the code accordingly. The real fix would be to embed "fd",
"listen", "connect", "mcast" and "udp" in a separate union. However
OptsVisitor's enum parser only supports the type=XXX QemuOpt instance as
union discriminator.

Signed-off-by: Laszlo Ersek <lersek@redhat.com>
---
 net/socket.c |  119 +++++++++++++++++++++-------------------------------------
 1 files changed, 43 insertions(+), 76 deletions(-)

Comments

Paolo Bonzini June 5, 2012, 9:02 p.m. UTC | #1 
Il 22/05/2012 12:45, Laszlo Ersek ha scritto:
> +    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
> +        sock->has_udp != 1) {

Please add a cast, this doesn't work with C99 bool.

Paolo

> +        error_report("exactly one of fd=, listen=, connect=, mcast= or udp="
> +                     " is required");
> +        return -1;
> +    }
Eric Blake June 5, 2012, 9:14 p.m. UTC | #2 
On 06/05/2012 03:02 PM, Paolo Bonzini wrote:
> Il 22/05/2012 12:45, Laszlo Ersek ha scritto:
>> +    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
>> +        sock->has_udp != 1) {
> 
> Please add a cast, this doesn't work with C99 bool.

Huh?  Where does C99 state that?

6.3.1.8 states that 'usual arithmetic conversions' consists of first
performing integer promotions (and _Bool promotes to int), then if both
sides of the operand are the same promoted type, no further conversion
is performed.

bool + bool

therefore undergoes integer promotion to

int + int

and the result can be > 1.

Perhaps you are thinking of the gnulib <stdbool.h> replacement, where
the usual arithmetic conventions of gnulib's replacement 'bool' do not
always coincide with C99 semantics?
Paolo Bonzini June 5, 2012, 9:27 p.m. UTC | #3 
Il 05/06/2012 23:14, Eric Blake ha scritto:
> On 06/05/2012 03:02 PM, Paolo Bonzini wrote:
>> Il 22/05/2012 12:45, Laszlo Ersek ha scritto:
>>> +    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
>>> +        sock->has_udp != 1) {
>>
>> Please add a cast, this doesn't work with C99 bool.
> 
> Huh?  Where does C99 state that?

Doh, I confused ++ and +.

Thanks for restoring some of my faith in X3J11.

Paolo
diff mbox

Patch

diff --git a/net/socket.c b/net/socket.c
index 563447d..e3cba20 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -586,101 +586,68 @@  static int net_socket_udp_init(VLANState *vlan,
     return 0;
 }
 
-int net_init_socket(QemuOpts *opts, const NetClientOptions *new_opts,
+int net_init_socket(QemuOpts *old_opts, const NetClientOptions *opts,
                     const char *name, VLANState *vlan)
 {
-    if (qemu_opt_get(opts, "fd")) {
-        int fd;
+    const NetdevSocketOptions *sock;
 
-        if (qemu_opt_get(opts, "listen") ||
-            qemu_opt_get(opts, "connect") ||
-            qemu_opt_get(opts, "mcast") ||
-            qemu_opt_get(opts, "localaddr")) {
-            error_report("listen=, connect=, mcast= and localaddr= is invalid with fd=");
-            return -1;
-        }
+    assert(opts->kind == NET_CLIENT_OPTIONS_KIND_SOCKET);
+    sock = opts->socket;
 
-        fd = net_handle_fd_param(cur_mon, qemu_opt_get(opts, "fd"));
-        if (fd == -1) {
-            return -1;
-        }
+    if (sock->has_fd + sock->has_listen + sock->has_connect + sock->has_mcast +
+        sock->has_udp != 1) {
+        error_report("exactly one of fd=, listen=, connect=, mcast= or udp="
+                     " is required");
+        return -1;
+    }
 
-        if (!net_socket_fd_init(vlan, "socket", name, fd, 1)) {
-            return -1;
-        }
-    } else if (qemu_opt_get(opts, "listen")) {
-        const char *listen;
-
-        if (qemu_opt_get(opts, "fd") ||
-            qemu_opt_get(opts, "connect") ||
-            qemu_opt_get(opts, "mcast") ||
-            qemu_opt_get(opts, "localaddr")) {
-            error_report("fd=, connect=, mcast= and localaddr= is invalid with listen=");
-            return -1;
-        }
+    if (sock->has_localaddr && !sock->has_mcast && !sock->has_udp) {
+        error_report("localaddr= is only valid with mcast= or udp=");
+        return -1;
+    }
 
-        listen = qemu_opt_get(opts, "listen");
+    if (sock->has_fd) {
+        int fd;
 
-        if (net_socket_listen_init(vlan, "socket", name, listen) == -1) {
-            return -1;
-        }
-    } else if (qemu_opt_get(opts, "connect")) {
-        const char *connect;
-
-        if (qemu_opt_get(opts, "fd") ||
-            qemu_opt_get(opts, "listen") ||
-            qemu_opt_get(opts, "mcast") ||
-            qemu_opt_get(opts, "localaddr")) {
-            error_report("fd=, listen=, mcast= and localaddr= is invalid with connect=");
+        fd = net_handle_fd_param(cur_mon, sock->fd);
+        if (fd == -1 || !net_socket_fd_init(vlan, "socket", name, fd, 1)) {
             return -1;
         }
+        return 0;
+    }
 
-        connect = qemu_opt_get(opts, "connect");
-
-        if (net_socket_connect_init(vlan, "socket", name, connect) == -1) {
+    if (sock->has_listen) {
+        if (net_socket_listen_init(vlan, "socket", name, sock->listen) == -1) {
             return -1;
         }
-    } else if (qemu_opt_get(opts, "mcast")) {
-        const char *mcast, *localaddr;
+        return 0;
+    }
 
-        if (qemu_opt_get(opts, "fd") ||
-            qemu_opt_get(opts, "connect") ||
-            qemu_opt_get(opts, "listen")) {
-            error_report("fd=, connect= and listen= is invalid with mcast=");
+    if (sock->has_connect) {
+        if (net_socket_connect_init(vlan, "socket", name, sock->connect) ==
+            -1) {
             return -1;
         }
+        return 0;
+    }
 
-        mcast = qemu_opt_get(opts, "mcast");
-        localaddr = qemu_opt_get(opts, "localaddr");
-
-        if (net_socket_mcast_init(vlan, "socket", name, mcast, localaddr) == -1) {
-            return -1;
-        }
-    } else if (qemu_opt_get(opts, "udp")) {
-        const char *udp, *localaddr;
-
-        if (qemu_opt_get(opts, "fd") ||
-            qemu_opt_get(opts, "connect") ||
-            qemu_opt_get(opts, "listen") ||
-            qemu_opt_get(opts, "mcast")) {
-            error_report("fd=, connect=, listen="
-                         " and mcast= is invalid with udp=");
+    if (sock->has_mcast) {
+        /* if sock->localaddr is missing, it has been initialized to "all bits
+         * zero" */
+        if (net_socket_mcast_init(vlan, "socket", name, sock->mcast,
+            sock->localaddr) == -1) {
             return -1;
         }
+        return 0;
+    }
 
-        udp = qemu_opt_get(opts, "udp");
-        localaddr = qemu_opt_get(opts, "localaddr");
-        if (localaddr == NULL) {
-                error_report("localaddr= is mandatory with udp=");
-                return -1;
-        }
-
-        if (net_socket_udp_init(vlan, "udp", name, udp, localaddr) == -1) {
-            return -1;
-        }
-    } else {
-        error_report("-socket requires fd=, listen=,"
-                     " connect=, mcast= or udp=");
+    assert(sock->has_udp);
+    if (!sock->has_localaddr) {
+        error_report("localaddr= is mandatory with udp=");
+        return -1;
+    }
+    if (net_socket_udp_init(vlan, "udp", name, sock->udp, sock->localaddr) ==
+        -1) {
         return -1;
     }
     return 0;