We have the following flaws with it: 1. Layring violation by modifying generic state directly in backends
2. Tricky generic logic: we should check, did backend set the generic state field, and fill it when not. Let's fix them all by making filename a private field with getter and setter. And move the "default logic" into getter. Signed-off-by: Vladimir Sementsov-Ogievskiy <[email protected]> --- chardev/char-pty.c | 4 +++- chardev/char-socket.c | 17 ++++++++--------- chardev/char.c | 8 ++------ hw/misc/ivshmem-pci.c | 4 ++-- include/chardev/char.h | 21 ++++++++++++++++++++- 5 files changed, 35 insertions(+), 19 deletions(-) diff --git a/chardev/char-pty.c b/chardev/char-pty.c index 047aade09e..f4294679be 100644 --- a/chardev/char-pty.c +++ b/chardev/char-pty.c @@ -336,6 +336,7 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp) int master_fd, slave_fd; char *name; char *path = backend->u.pty.data->path; + g_autofree char *filename = NULL; s = PTY_CHARDEV(chr); @@ -351,7 +352,8 @@ static bool pty_chr_open(Chardev *chr, ChardevBackend *backend, Error **errp) return false; } - chr->filename = g_strdup_printf("pty:%s", s->pty_name); + filename = g_strdup_printf("pty:%s", s->pty_name); + qemu_chr_set_filename(chr, filename); qemu_printf("char device redirected to %s (label %s)\n", s->pty_name, chr->label); diff --git a/chardev/char-socket.c b/chardev/char-socket.c index 31c9acd164..9387760009 100644 --- a/chardev/char-socket.c +++ b/chardev/char-socket.c @@ -384,8 +384,7 @@ static void tcp_chr_free_connection(Chardev *chr) s->sioc = NULL; object_unref(OBJECT(s->ioc)); s->ioc = NULL; - g_free(chr->filename); - chr->filename = NULL; + qemu_chr_set_filename(chr, NULL); tcp_chr_change_state(s, TCP_CHARDEV_STATE_DISCONNECTED); } @@ -443,11 +442,11 @@ static void update_disconnected_filename(SocketChardev *s) { Chardev *chr = CHARDEV(s); - g_free(chr->filename); if (s->addr) { - chr->filename = qemu_chr_socket_address(s, "disconnected:"); + g_autofree char *filename = qemu_chr_socket_address(s, "disconnected:"); + qemu_chr_set_filename(chr, filename); } else { - chr->filename = g_strdup("disconnected:socket"); + qemu_chr_set_filename(chr, "disconnected:socket"); } } @@ -638,9 +637,9 @@ static void tcp_chr_connect(void *opaque) { Chardev *chr = CHARDEV(opaque); SocketChardev *s = SOCKET_CHARDEV(opaque); + g_autofree char *filename = qemu_chr_compute_filename(s); - g_free(chr->filename); - chr->filename = qemu_chr_compute_filename(s); + qemu_chr_set_filename(chr, filename); tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTED); update_ioc_handlers(s); @@ -1000,8 +999,8 @@ static void tcp_chr_accept_server_sync(Chardev *chr) { SocketChardev *s = SOCKET_CHARDEV(chr); QIOChannelSocket *sioc; - info_report("QEMU waiting for connection on: %s", - chr->filename); + g_autofree char *filename = qemu_chr_get_filename(chr); + info_report("QEMU waiting for connection on: %s", filename); tcp_chr_change_state(s, TCP_CHARDEV_STATE_CONNECTING); sioc = qio_net_listener_wait_client(s->listener); tcp_chr_set_client_ioc_name(chr, sioc); diff --git a/chardev/char.c b/chardev/char.c index 0dc792b88f..bdd907f015 100644 --- a/chardev/char.c +++ b/chardev/char.c @@ -309,7 +309,7 @@ static void char_finalize(Object *obj) if (chr->fe) { chr->fe->chr = NULL; } - g_free(chr->filename); + qemu_chr_set_filename(chr, NULL); g_free(chr->label); if (chr->logfd != -1) { close(chr->logfd); @@ -796,7 +796,7 @@ static int qmp_query_chardev_foreach(Object *obj, void *data) ChardevInfo *value = g_malloc0(sizeof(*value)); value->label = g_strdup(chr->label); - value->filename = g_strdup(chr->filename); + value->filename = qemu_chr_get_filename(chr); value->frontend_open = chr->fe && chr->fe->fe_is_open; QAPI_LIST_PREPEND(*list, value); @@ -1025,10 +1025,6 @@ static Chardev *chardev_new(const char *id, const char *typename, return NULL; } - if (!chr->filename) { - chr->filename = g_strdup(typename + 8); - } - return chr; } diff --git a/hw/misc/ivshmem-pci.c b/hw/misc/ivshmem-pci.c index 636d0b83de..2c7b987241 100644 --- a/hw/misc/ivshmem-pci.c +++ b/hw/misc/ivshmem-pci.c @@ -873,10 +873,10 @@ static void ivshmem_common_realize(PCIDevice *dev, Error **errp) host_memory_backend_set_mapped(s->hostmem, true); } else { Chardev *chr = qemu_chr_fe_get_driver(&s->server_chr); + char *filename = qemu_chr_get_filename(chr); assert(chr); - IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", - chr->filename); + IVSHMEM_DPRINTF("using shared memory server (socket = %s)\n", filename); /* we allocate enough space for 16 peers and grow as needed */ resize_peers(s, 16); diff --git a/include/chardev/char.h b/include/chardev/char.h index d36e50b99e..ffeb4a4e3b 100644 --- a/include/chardev/char.h +++ b/include/chardev/char.h @@ -62,7 +62,7 @@ struct Chardev { QemuMutex chr_write_lock; CharFrontend *fe; char *label; - char *filename; + char *_filename; int logfd; int be_open; /* used to coordinate the chardev-change special-case: */ @@ -72,6 +72,25 @@ struct Chardev { DECLARE_BITMAP(features, QEMU_CHAR_FEATURE_LAST); }; +static inline char *qemu_chr_get_filename(Chardev *chr) +{ + const char *typename; + + if (chr->_filename) { + return g_strdup(chr->_filename); + } + + typename = object_get_typename(OBJECT(chr)); + assert(g_str_has_prefix(typename, "chardev-")); + return g_strdup(typename + 8); +} + +static inline void qemu_chr_set_filename(Chardev *chr, const char *filename) +{ + g_free(chr->_filename); + chr->_filename = g_strdup(filename); +} + /** * qemu_chr_new_from_opts: * @opts: see qemu-config.c for a list of valid options -- 2.48.1
