Let's fix this before we bake in a painful API. Since we know that we have exactly one non-negative fd on success, we might as well return the fd directly instead of forcing the user to pass in a pointer. Fix a memory leak I found while reviewing. * include/libvirt/libvirt.h.in (virDomainOpenGraphicsFD): Drop unneeded parameter. * src/driver.h (virDrvDomainOpenGraphicsFD): Likewise. * src/libvirt.c (virDomainOpenGraphicsFD): Adjust interface to return fd directly. * daemon/remote.c (remoteDispatchDomainOpenGraphicsFd): Adjust semantics. * src/qemu/qemu_driver.c (qemuDomainOpenGraphicsFD): Likewise. * src/remote/remote_driver.c (remoteDomainOpenGraphicsFD): Likewise, and plug memory leak. Signed-off-by: Eric Blake <eblake@redhat.com> --- daemon/remote.c | 9 +++++---- include/libvirt/libvirt.h.in | 1 - src/driver.h | 1 - src/libvirt.c | 14 ++++++-------- src/qemu/qemu_driver.c | 3 +-- src/remote/remote_driver.c | 17 +++++++++++------ 6 files changed, 23 insertions(+), 22 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 27d1aa8..940dcfa 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -4419,10 +4419,9 @@ remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED, if (!(dom = get_nonnull_domain(priv->conn, args->dom))) goto cleanup; - if (virDomainOpenGraphicsFD(dom, - args->idx, - &fd, - args->flags) < 0) + if ((fd = virDomainOpenGraphicsFD(dom, + args->idx, + args->flags)) < 0) goto cleanup; if (virNetMessageAddFD(msg, fd) < 0) @@ -4442,6 +4441,8 @@ remoteDispatchDomainOpenGraphicsFd(virNetServerPtr server ATTRIBUTE_UNUSED, virDomainFree(dom); return rv; } + + static int remoteDispatchDomainGetInterfaceParameters(virNetServerPtr server ATTRIBUTE_UNUSED, virNetServerClientPtr client ATTRIBUTE_UNUSED, diff --git a/include/libvirt/libvirt.h.in b/include/libvirt/libvirt.h.in index 2a108b8..e79c9ad 100644 --- a/include/libvirt/libvirt.h.in +++ b/include/libvirt/libvirt.h.in @@ -5401,7 +5401,6 @@ int virDomainOpenGraphics(virDomainPtr dom, int virDomainOpenGraphicsFD(virDomainPtr dom, unsigned int idx, - int *fd, unsigned int flags); int virDomainInjectNMI(virDomainPtr domain, unsigned int flags); diff --git a/src/driver.h b/src/driver.h index cd136ba..a7366e4 100644 --- a/src/driver.h +++ b/src/driver.h @@ -890,7 +890,6 @@ typedef int typedef int (*virDrvDomainOpenGraphicsFD)(virDomainPtr dom, unsigned int idx, - int *fd, unsigned int flags); typedef int diff --git a/src/libvirt.c b/src/libvirt.c index 772cc0d..7a3f01a 100644 --- a/src/libvirt.c +++ b/src/libvirt.c @@ -20305,11 +20305,10 @@ virDomainOpenGraphics(virDomainPtr dom, * virDomainOpenGraphicsFD: * @dom: pointer to domain object * @idx: index of graphics config to open - * @fd: returned file descriptor * @flags: bitwise-OR of virDomainOpenGraphicsFlags * * This will create a socket pair connected to the graphics backend of @dom. - * One socket will be returned as @fd. + * One end of the socket will be returned on success. * If @dom has multiple graphics backends configured, then @idx will determine * which one is opened, starting from @idx 0. * @@ -20320,21 +20319,18 @@ virDomainOpenGraphics(virDomainPtr dom, * libvirt hypervisor, over a UNIX domain socket. Attempts * to use this method over a TCP connection will always fail * - * Returns 0 on success, -1 on failure + * Returns an fd on success, -1 on failure */ int virDomainOpenGraphicsFD(virDomainPtr dom, unsigned int idx, - int *fd, unsigned int flags) { - VIR_DOMAIN_DEBUG(dom, "idx=%u, fd=%p, flags=%x", - idx, fd, flags); + VIR_DOMAIN_DEBUG(dom, "idx=%u, flags=%x", idx, flags); virResetLastError(); virCheckDomainReturn(dom, -1); - virCheckNonNullArgGoto(fd, error); virCheckReadOnlyGoto(dom->conn->flags, error); @@ -20347,7 +20343,7 @@ virDomainOpenGraphicsFD(virDomainPtr dom, if (dom->conn->driver->domainOpenGraphicsFD) { int ret; - ret = dom->conn->driver->domainOpenGraphicsFD(dom, idx, fd, flags); + ret = dom->conn->driver->domainOpenGraphicsFD(dom, idx, flags); if (ret < 0) goto error; return ret; @@ -20359,6 +20355,8 @@ virDomainOpenGraphicsFD(virDomainPtr dom, virDispatchError(dom->conn); return -1; } + + /** * virConnectSetKeepAlive: * @conn: pointer to a hypervisor connection diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 5ff2059..57c999c 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15808,7 +15808,6 @@ qemuDomainOpenGraphics(virDomainPtr dom, static int qemuDomainOpenGraphicsFD(virDomainPtr dom, unsigned int idx, - int *fd, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; @@ -15871,7 +15870,7 @@ qemuDomainOpenGraphicsFD(virDomainPtr dom, if (!qemuDomainObjEndJob(driver, vm)) vm = NULL; - *fd = pair[0]; + ret = pair[0]; cleanup: if (ret < 0) { diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index e01216a..e949223 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -6448,7 +6448,6 @@ remoteDomainOpenGraphics(virDomainPtr dom, static int remoteDomainOpenGraphicsFD(virDomainPtr dom, unsigned int idx, - int *fd, unsigned int flags) { int rv = -1; @@ -6472,15 +6471,21 @@ remoteDomainOpenGraphicsFD(virDomainPtr dom, goto done; if (fdoutlen != 1) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("no file descriptor received")); + if (fdoutlen) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("too many file descriptors received")); + while (fdoutlen) + VIR_FORCE_CLOSE(fdout[--fdoutlen]); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("no file descriptor received")); + } goto done; } - *fd = fdout[0]; - - rv = 0; + rv = fdout[0]; done: + VIR_FREE(fdout); remoteDriverUnlock(priv); return rv; -- 1.9.3