| Message ID | 20201029133833.3450220-10-armbru@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | sockets: Attempt to drain the abstract socket swamp | expand |
On 10/29/20 8:38 AM, Markus Armbruster wrote: > Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket > support" neglected to update qemu_chr_socket_address(). It shows > shows neither @abstract nor @tight. Fix that. > > Signed-off-by: Markus Armbruster <armbru@redhat.com> > --- > chardev/char-socket.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > +++ b/chardev/char-socket.c > @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) > s->is_listen ? ",server" : ""); > break; > case SOCKET_ADDRESS_TYPE_UNIX: > - return g_strdup_printf("%sunix:%s%s", prefix, > + { > + UnixSocketAddress *sa = &s->addr->u.q_unix; > + > + return g_strdup_printf("%sunix:%s%s%s%s", prefix, > s->addr->u.q_unix.path, > + sa->has_abstract && sa->abstract > + ? ",abstract" : "", > + sa->has_tight && sa->tight > + ? ",tight" : "", Why are we appending ',tight' when it is not abstract? tight only makes a difference for abstract sockets, so omitting it for normal sockets makes more sense. Or put another way, why are we using 2 bools to represent three sensible states, instead of a single 3-state enum? > s->is_listen ? ",server" : ""); > break; > + } > case SOCKET_ADDRESS_TYPE_FD: > return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, > s->is_listen ? ",server" : ""); > -- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3226 Virtualization: qemu.org | libvirt.org
Eric Blake <eblake@redhat.com> writes: > On 10/29/20 8:38 AM, Markus Armbruster wrote: >> Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket >> support" neglected to update qemu_chr_socket_address(). It shows >> shows neither @abstract nor @tight. Fix that. >> >> Signed-off-by: Markus Armbruster <armbru@redhat.com> >> --- >> chardev/char-socket.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> > >> +++ b/chardev/char-socket.c >> @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) >> s->is_listen ? ",server" : ""); >> break; >> case SOCKET_ADDRESS_TYPE_UNIX: >> - return g_strdup_printf("%sunix:%s%s", prefix, >> + { >> + UnixSocketAddress *sa = &s->addr->u.q_unix; >> + >> + return g_strdup_printf("%sunix:%s%s%s%s", prefix, >> s->addr->u.q_unix.path, >> + sa->has_abstract && sa->abstract >> + ? ",abstract" : "", >> + sa->has_tight && sa->tight >> + ? ",tight" : "", > > Why are we appending ',tight' when it is not abstract? tight only makes > a difference for abstract sockets, so omitting it for normal sockets > makes more sense. We don't bother to reject @tight when @abstract is false. Not bothering to suppress it here is consistently careless. I'm trying to make this pig less wrong, I'm not trying to make it less ugly. > Or put another way, why are we using 2 bools to represent three sensible > states, instead of a single 3-state enum? Because the QAPI interface got merged without proper review by the QAPI maintainers? >> s->is_listen ? ",server" : ""); >> break; >> + } >> case SOCKET_ADDRESS_TYPE_FD: >> return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, >> s->is_listen ? ",server" : ""); >>
diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 1ee5a8c295..dc1cf86ecf 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -443,10 +443,18 @@ static char *qemu_chr_socket_address(SocketChardev *s, const char *prefix) s->is_listen ? ",server" : ""); break; case SOCKET_ADDRESS_TYPE_UNIX: - return g_strdup_printf("%sunix:%s%s", prefix, + { + UnixSocketAddress *sa = &s->addr->u.q_unix; + + return g_strdup_printf("%sunix:%s%s%s%s", prefix, s->addr->u.q_unix.path, + sa->has_abstract && sa->abstract + ? ",abstract" : "", + sa->has_tight && sa->tight + ? ",tight" : "", s->is_listen ? ",server" : ""); break; + } case SOCKET_ADDRESS_TYPE_FD: return g_strdup_printf("%sfd:%s%s", prefix, s->addr->u.fd.str, s->is_listen ? ",server" : "");
Commit 776b97d360 "qemu-sockets: add abstract UNIX domain socket support" neglected to update qemu_chr_socket_address(). It shows shows neither @abstract nor @tight. Fix that. Signed-off-by: Markus Armbruster <armbru@redhat.com> --- chardev/char-socket.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)