From 60efffa41b34cc299fed65b67e2c2641846d592b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= Date: Thu, 27 Aug 2020 13:27:00 +0100 Subject: [PATCH] monitor: simplify functions for getting a dup'd fdset entry MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently code has to call monitor_fdset_get_fd, then dup the return fd, and then add the duplicate FD back into the fdset. This dance is overly verbose for the caller and introduces extra failure modes which can be avoided by folding all the logic into monitor_fdset_dup_fd_add and removing monitor_fdset_get_fd entirely. Signed-off-by: Daniel P. Berrangé --- include/monitor/monitor.h | 3 +- include/qemu/osdep.h | 1 + monitor/misc.c | 58 +++++++++++++++++---------------------- stubs/fdset.c | 8 ++---- util/osdep.c | 19 ++----------- 5 files changed, 32 insertions(+), 57 deletions(-) diff --git a/include/monitor/monitor.h b/include/monitor/monitor.h index 1018d754a6..c0170773d4 100644 --- a/include/monitor/monitor.h +++ b/include/monitor/monitor.h @@ -43,8 +43,7 @@ int monitor_read_password(MonitorHMP *mon, ReadLineFunc *readline_func, AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, bool has_opaque, const char *opaque, Error **errp); -int monitor_fdset_get_fd(int64_t fdset_id, int flags); -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd); +int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags); void monitor_fdset_dup_fd_remove(int dup_fd); int64_t monitor_fdset_dup_fd_find(int dup_fd); diff --git a/include/qemu/osdep.h b/include/qemu/osdep.h index 412962d91a..66ee5bc45d 100644 --- a/include/qemu/osdep.h +++ b/include/qemu/osdep.h @@ -501,6 +501,7 @@ int qemu_open(const char *name, int flags, ...); int qemu_close(int fd); int qemu_unlink(const char *name); #ifndef _WIN32 +int qemu_dup_flags(int fd, int flags); int qemu_dup(int fd); #endif int qemu_lock_fd(int fd, int64_t start, int64_t len, bool exclusive); diff --git a/monitor/misc.c b/monitor/misc.c index e847b58a8c..0b1b9b196c 100644 --- a/monitor/misc.c +++ b/monitor/misc.c @@ -1547,69 +1547,61 @@ AddfdInfo *monitor_fdset_add_fd(int fd, bool has_fdset_id, int64_t fdset_id, return fdinfo; } -int monitor_fdset_get_fd(int64_t fdset_id, int flags) +int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags) { #ifdef _WIN32 return -ENOENT; #else MonFdset *mon_fdset; - MonFdsetFd *mon_fdset_fd; - int mon_fd_flags; - int ret; qemu_mutex_lock(&mon_fdsets_lock); QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { + MonFdsetFd *mon_fdset_fd; + MonFdsetFd *mon_fdset_fd_dup; + int fd = -1; + int dup_fd; + int mon_fd_flags; + if (mon_fdset->id != fdset_id) { continue; } + QLIST_FOREACH(mon_fdset_fd, &mon_fdset->fds, next) { mon_fd_flags = fcntl(mon_fdset_fd->fd, F_GETFL); if (mon_fd_flags == -1) { - ret = -errno; - goto out; + qemu_mutex_unlock(&mon_fdsets_lock); + return -1; } if ((flags & O_ACCMODE) == (mon_fd_flags & O_ACCMODE)) { - ret = mon_fdset_fd->fd; - goto out; + fd = mon_fdset_fd->fd; + break; } } - ret = -EACCES; - goto out; - } - ret = -ENOENT; -out: - qemu_mutex_unlock(&mon_fdsets_lock); - return ret; -#endif -} - -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) -{ - MonFdset *mon_fdset; - MonFdsetFd *mon_fdset_fd_dup; - - qemu_mutex_lock(&mon_fdsets_lock); - QLIST_FOREACH(mon_fdset, &mon_fdsets, next) { - if (mon_fdset->id != fdset_id) { - continue; + if (fd == -1) { + qemu_mutex_unlock(&mon_fdsets_lock); + errno = EACCES; + return -1; } - QLIST_FOREACH(mon_fdset_fd_dup, &mon_fdset->dup_fds, next) { - if (mon_fdset_fd_dup->fd == dup_fd) { - goto err; - } + + dup_fd = qemu_dup_flags(fd, flags); + if (dup_fd == -1) { + qemu_mutex_unlock(&mon_fdsets_lock); + return -1; } + mon_fdset_fd_dup = g_malloc0(sizeof(*mon_fdset_fd_dup)); mon_fdset_fd_dup->fd = dup_fd; QLIST_INSERT_HEAD(&mon_fdset->dup_fds, mon_fdset_fd_dup, next); qemu_mutex_unlock(&mon_fdsets_lock); - return 0; + return dup_fd; } -err: qemu_mutex_unlock(&mon_fdsets_lock); + errno = ENOENT; return -1; +#endif } static int64_t monitor_fdset_dup_fd_find_remove(int dup_fd, bool remove) diff --git a/stubs/fdset.c b/stubs/fdset.c index 67dd5e1d34..56b3663d58 100644 --- a/stubs/fdset.c +++ b/stubs/fdset.c @@ -1,8 +1,9 @@ #include "qemu/osdep.h" #include "monitor/monitor.h" -int monitor_fdset_dup_fd_add(int64_t fdset_id, int dup_fd) +int monitor_fdset_dup_fd_add(int64_t fdset_id, int flags) { + errno = ENOSYS; return -1; } @@ -11,11 +12,6 @@ int64_t monitor_fdset_dup_fd_find(int dup_fd) return -1; } -int monitor_fdset_get_fd(int64_t fdset_id, int flags) -{ - return -ENOENT; -} - void monitor_fdset_dup_fd_remove(int dupfd) { } diff --git a/util/osdep.c b/util/osdep.c index 4829c07ff6..3d94b4d732 100644 --- a/util/osdep.c +++ b/util/osdep.c @@ -122,7 +122,7 @@ static int fcntl_op_getlk = -1; /* * Dups an fd and sets the flags */ -static int qemu_dup_flags(int fd, int flags) +int qemu_dup_flags(int fd, int flags) { int ret; int serrno; @@ -293,7 +293,7 @@ int qemu_open(const char *name, int flags, ...) /* Attempt dup of fd from fd set */ if (strstart(name, "/dev/fdset/", &fdset_id_str)) { int64_t fdset_id; - int fd, dupfd; + int dupfd; fdset_id = qemu_parse_fdset(fdset_id_str); if (fdset_id == -1) { @@ -301,24 +301,11 @@ int qemu_open(const char *name, int flags, ...) return -1; } - fd = monitor_fdset_get_fd(fdset_id, flags); - if (fd < 0) { - errno = -fd; - return -1; - } - - dupfd = qemu_dup_flags(fd, flags); + dupfd = monitor_fdset_dup_fd_add(fdset_id, flags); if (dupfd == -1) { return -1; } - ret = monitor_fdset_dup_fd_add(fdset_id, dupfd); - if (ret == -1) { - close(dupfd); - errno = EINVAL; - return -1; - } - return dupfd; } #endif