diff mbox

[v2,3/5] qemu-char: Convert udp backend to QAPI

Message ID 1409653457-27863-4-git-send-email-peter.maydell@linaro.org
State Accepted
Commit 90a14bfe52166e5a85fff4e20532a22ae21dc6a7
Headers show

Commit Message

Peter Maydell Sept. 2, 2014, 10:24 a.m. UTC 
Convert the udp char backend to the new style QAPI framework.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
 qemu-char.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------------
 1 file changed, 54 insertions(+), 15 deletions(-)

Comments

Eric Blake Sept. 2, 2014, 8:04 p.m. UTC | #1 
On 09/02/2014 04:24 AM, Peter Maydell wrote:
> Convert the udp char backend to the new style QAPI framework.
> 
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
>  qemu-char.c | 69 +++++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 54 insertions(+), 15 deletions(-)
> 

> +
> +    if (host == NULL || strlen(host) == 0) {
> +        host = "localhost";
> +    }
> +    if (port == NULL || strlen(port) == 0) {
> +        error_setg(errp, "chardev: udp: remote port not specified");

In the common case of these strings being non-empty, you end up having
to hunt for the end of the string only to then throw that information
away.  Rather than 'strlen(foo) == 0)', it's slightly faster to check
'*foo' for being a non-NUL byte.
Peter Maydell Sept. 2, 2014, 8:30 p.m. UTC | #2 
On 2 September 2014 21:04, Eric Blake <eblake@redhat.com> wrote:
> On 09/02/2014 04:24 AM, Peter Maydell wrote:
>> +
>> +    if (host == NULL || strlen(host) == 0) {
>> +        host = "localhost";
>> +    }
>> +    if (port == NULL || strlen(port) == 0) {
>> +        error_setg(errp, "chardev: udp: remote port not specified");
>
> In the common case of these strings being non-empty, you end up having
> to hunt for the end of the string only to then throw that information
> away.  Rather than 'strlen(foo) == 0)', it's slightly faster to check
> '*foo' for being a non-NUL byte.

I think that's a lot less clear to read, and in fact the
compiler is entirely capable of turning "strlen(x) == 0"
into "check whether *x is 0":

mnementh$ cat /tmp/zz9.c
#include <string.h>

int is_empty_string(const char *s)
{
    return (strlen(s) == 0);
}
mnementh$ gcc -g -Wall -O2 -o /tmp/zz9.o -c /tmp/zz9.c
mnementh$ objdump --disassemble /tmp/zz9.o

/tmp/zz9.o:     file format elf32-i386


Disassembly of section .text:

00000000 <is_empty_string>:
   0:    8b 44 24 04              mov    0x4(%esp),%eax
   4:    80 38 00                 cmpb   $0x0,(%eax)
   7:    0f 94 c0                 sete   %al
   a:    0f b6 c0                 movzbl %al,%eax
   d:    c3                       ret

thanks
-- PMM
diff mbox

Patch

diff --git a/qemu-char.c b/qemu-char.c
index 4b99382..97a839e 100644
--- a/qemu-char.c
+++ b/qemu-char.c
@@ -2383,20 +2383,6 @@  static CharDriverState *qemu_chr_open_udp_fd(int fd)
     return chr;
 }
 
-static CharDriverState *qemu_chr_open_udp(QemuOpts *opts)
-{
-    Error *local_err = NULL;
-    int fd = -1;
-
-    fd = inet_dgram_opts(opts, &local_err);
-    if (fd < 0) {
-        qerror_report_err(local_err);
-        error_free(local_err);
-        return NULL;
-    }
-    return qemu_chr_open_udp_fd(fd);
-}
-
 /***********************************************************/
 /* TCP Net console */
 
@@ -3447,6 +3433,58 @@  static void qemu_chr_parse_socket(QemuOpts *opts, ChardevBackend *backend,
     backend->socket->addr = addr;
 }
 
+static void qemu_chr_parse_udp(QemuOpts *opts, ChardevBackend *backend,
+                               Error **errp)
+{
+    const char *host = qemu_opt_get(opts, "host");
+    const char *port = qemu_opt_get(opts, "port");
+    const char *localaddr = qemu_opt_get(opts, "localaddr");
+    const char *localport = qemu_opt_get(opts, "localport");
+    bool has_local = false;
+    SocketAddress *addr;
+
+    if (host == NULL || strlen(host) == 0) {
+        host = "localhost";
+    }
+    if (port == NULL || strlen(port) == 0) {
+        error_setg(errp, "chardev: udp: remote port not specified");
+        return;
+    }
+    if (localport == NULL || strlen(localport) == 0) {
+        localport = "0";
+    } else {
+        has_local = true;
+    }
+    if (localaddr == NULL || strlen(localaddr) == 0) {
+        localaddr = "";
+    } else {
+        has_local = true;
+    }
+
+    backend->udp = g_new0(ChardevUdp, 1);
+
+    addr = g_new0(SocketAddress, 1);
+    addr->kind = SOCKET_ADDRESS_KIND_INET;
+    addr->inet = g_new0(InetSocketAddress, 1);
+    addr->inet->host = g_strdup(host);
+    addr->inet->port = g_strdup(port);
+    addr->inet->has_ipv4 = qemu_opt_get(opts, "ipv4");
+    addr->inet->ipv4 = qemu_opt_get_bool(opts, "ipv4", 0);
+    addr->inet->has_ipv6 = qemu_opt_get(opts, "ipv6");
+    addr->inet->ipv6 = qemu_opt_get_bool(opts, "ipv6", 0);
+    backend->udp->remote = addr;
+
+    if (has_local) {
+        backend->udp->has_local = true;
+        addr = g_new0(SocketAddress, 1);
+        addr->kind = SOCKET_ADDRESS_KIND_INET;
+        addr->inet = g_new0(InetSocketAddress, 1);
+        addr->inet->host = g_strdup(localaddr);
+        addr->inet->port = g_strdup(localport);
+        backend->udp->local = addr;
+    }
+}
+
 typedef struct CharDriver {
     const char *name;
     /* old, pre qapi */
@@ -4137,7 +4175,8 @@  static void register_types(void)
     register_char_driver_qapi("null", CHARDEV_BACKEND_KIND_NULL, NULL);
     register_char_driver_qapi("socket", CHARDEV_BACKEND_KIND_SOCKET,
                               qemu_chr_parse_socket);
-    register_char_driver("udp", qemu_chr_open_udp);
+    register_char_driver_qapi("udp", CHARDEV_BACKEND_KIND_UDP,
+                              qemu_chr_parse_udp);
     register_char_driver_qapi("ringbuf", CHARDEV_BACKEND_KIND_RINGBUF,
                               qemu_chr_parse_ringbuf);
     register_char_driver_qapi("file", CHARDEV_BACKEND_KIND_FILE,