qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight


From: Markus Armbruster
Subject: [PATCH 07/11] sockets: Fix default of UnixSocketAddress member @tight
Date: Thu, 29 Oct 2020 14:38:29 +0100

QMP chardev-add defaults absent member @tight to false instead of
true.  HMP chardev-add and CLI -chardev correctly default to true.

The previous commit demonstrated that socket_listen() and
socket_connect() are broken for absent @tight.  That explains why QMP
is broken, but not why HMP and CLI work.  We need to dig deeper.

An optional bool member of a QAPI struct can be false, true, or
absent.  In C, we have:

            has_MEMBER    MEMBER
    false         true     false
    true          true     false
    absent       false  false/ignore

When has_MEMBER is false, MEMBER should be set to false on write, and
ignored on read.

unix_listen_saddr() and unix_connect_saddr() use member @tight without
checking @has_tight.  This is wrong.

When @tight was set to false as it should be, absent @tight defaults
to false.  Wrong, it should default to true.  This is what breaks QMP.

There is one exception: qemu_chr_parse_socket() leaves @has_tight
false when it sets @tight.  Wrong, but the wrongs cancel out.  This is
why HMP and CLI work.  Same for @has_abstract.

Fix unix_listen_saddr() and unix_connect_saddr() to default absent
@tight to true.

Fix qemu_chr_parse_socket() to set @has_tight and @has_abstract.

Fixes: 776b97d3605ed0fc94443048fdf988c7725e38a9
Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
 chardev/char-socket.c     | 2 ++
 tests/test-util-sockets.c | 6 +++---
 util/qemu-sockets.c       | 4 ++--
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/chardev/char-socket.c b/chardev/char-socket.c
index 95e45812d5..1ee5a8c295 100644
--- a/chardev/char-socket.c
+++ b/chardev/char-socket.c
@@ -1439,7 +1439,9 @@ static void qemu_chr_parse_socket(QemuOpts *opts, 
ChardevBackend *backend,
         addr->type = SOCKET_ADDRESS_LEGACY_KIND_UNIX;
         q_unix = addr->u.q_unix.data = g_new0(UnixSocketAddress, 1);
         q_unix->path = g_strdup(path);
+        q_unix->has_tight = true;
         q_unix->tight = tight;
+        q_unix->has_abstract = true;
         q_unix->abstract = abstract;
     } else if (host) {
         addr->type = SOCKET_ADDRESS_LEGACY_KIND_INET;
diff --git a/tests/test-util-sockets.c b/tests/test-util-sockets.c
index f8b6586e70..7ecf95579b 100644
--- a/tests/test-util-sockets.c
+++ b/tests/test-util-sockets.c
@@ -294,13 +294,13 @@ static void test_socket_unix_abstract(void)
     abstract_socket_matrix_row matrix[ABSTRACT_SOCKET_VARIANTS] = {
         { &addr,
           { &addr_tight, &addr_padded, &addr },
-          { false /* BUG */, true /* BUG */, true } },
+          { true, false, true } },
         { &addr_tight,
           { &addr_padded, &addr, &addr_tight },
-          { false, false /* BUG */, true } },
+          { false, true, true } },
         { &addr_padded,
           { &addr, &addr_tight, &addr_padded },
-          { true /* BUG */, false, true } }
+          { false, false, true } }
     };
     int i;
 
diff --git a/util/qemu-sockets.c b/util/qemu-sockets.c
index 05e5c73f9d..c802d5aa0a 100644
--- a/util/qemu-sockets.c
+++ b/util/qemu-sockets.c
@@ -919,7 +919,7 @@ static int unix_listen_saddr(UnixSocketAddress *saddr,
     if (saddr->abstract) {
         un.sun_path[0] = '\0';
         memcpy(&un.sun_path[1], path, pathlen);
-        if (saddr->tight) {
+        if (!saddr->has_tight || saddr->tight) {
             addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
         }
     } else {
@@ -979,7 +979,7 @@ static int unix_connect_saddr(UnixSocketAddress *saddr, 
Error **errp)
     if (saddr->abstract) {
         un.sun_path[0] = '\0';
         memcpy(&un.sun_path[1], saddr->path, pathlen);
-        if (saddr->tight) {
+        if (!saddr->has_tight || saddr->tight) {
             addrlen = offsetof(struct sockaddr_un, sun_path) + 1 + pathlen;
         }
     } else {
-- 
2.26.2




reply via email to

[Prev in Thread] Current Thread [Next in Thread]