We want to switch from qemu_socket_set_block() to newer
qemu_set_blocking(), which provides return status of operation,
to handle errors.
Still, we want to keep qio_channel_socket_readv() interface clean,
as currently it allocate @fds only on success.
So, in case of error, we should close all incoming fds and keep
user's @fds untouched or zero.
Let's make separate functions qio_channel_handle_fds() and
qio_channel_cleanup_fds(), to achieve what we want.
Signed-off-by: Vladimir Sementsov-Ogievskiy <vsementsov@yandex-team.ru>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
include/io/channel.h | 8 +++---
io/channel-socket.c | 67 +++++++++++++++++++++++++++++++++-----------
2 files changed, 55 insertions(+), 20 deletions(-)
diff --git a/include/io/channel.h b/include/io/channel.h
index 999a8f5f23..0f25ae0069 100644
--- a/include/io/channel.h
+++ b/include/io/channel.h
@@ -124,8 +124,8 @@ struct QIOChannelClass {
* incoming fds are set BLOCKING (unless
* QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING flag is set) and
* CLOEXEC (if available).
- * @fds and @nfds are set only on success path, and untouched
- * in case of errors.
+ * @fds and @nfds are set only on success path. Still, setting
+ * @fds and @nfds to zero is acceptable on failure path.
*/
ssize_t (*io_readv)(QIOChannel *ioc,
const struct iovec *iov,
@@ -246,8 +246,8 @@ void qio_channel_set_name(QIOChannel *ioc,
* to call close() on each file descriptor and to
* call g_free() on the array pointer in @fds.
* @fds allocated and set (and @nfds is set too)
- * _only_ on success path. These parameters are
- * untouched in case of errors.
+ * _only_ on success path. Still, @fds and @nfds
+ * may be set to zero on failure path.
* qio_channel_readv_full() guarantees that all
* incoming fds are set BLOCKING (unless
* QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING flag
diff --git a/io/channel-socket.c b/io/channel-socket.c
index f7e3cb9742..e53d9ac76f 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -464,8 +464,7 @@ static void qio_channel_socket_finalize(Object *obj)
#ifndef WIN32
static void qio_channel_socket_copy_fds(struct msghdr *msg,
- int **fds, size_t *nfds,
- bool preserve_blocking)
+ int **fds, size_t *nfds)
{
struct cmsghdr *cmsg;
@@ -473,7 +472,7 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
*fds = NULL;
for (cmsg = CMSG_FIRSTHDR(msg); cmsg; cmsg = CMSG_NXTHDR(msg, cmsg)) {
- int fd_size, i;
+ int fd_size;
int gotfds;
if (cmsg->cmsg_len < CMSG_LEN(sizeof(int)) ||
@@ -491,24 +490,53 @@ static void qio_channel_socket_copy_fds(struct msghdr *msg,
gotfds = fd_size / sizeof(int);
*fds = g_renew(int, *fds, *nfds + gotfds);
memcpy(*fds + *nfds, CMSG_DATA(cmsg), fd_size);
+ *nfds += gotfds;
+ }
+}
- for (i = 0; i < gotfds; i++) {
- int fd = (*fds)[*nfds + i];
- if (fd < 0) {
- continue;
- }
+static bool qio_channel_handle_fds(int *fds, size_t nfds,
+ bool preserve_blocking, Error **errp)
+{
+ int *end = fds + nfds, *fd;
+
+#ifdef MSG_CMSG_CLOEXEC
+ if (preserve_blocking) {
+ /* Nothing to do */
+ return true;
+ }
+#endif
- if (!preserve_blocking) {
- /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
- qemu_socket_set_block(fd);
+ for (fd = fds; fd != end; fd++) {
+ if (*fd < 0) {
+ continue;
+ }
+
+ if (!preserve_blocking) {
+ /* O_NONBLOCK is preserved across SCM_RIGHTS so reset it */
+ if (!qemu_set_blocking(*fd, true, errp)) {
+ return false;
}
+ }
#ifndef MSG_CMSG_CLOEXEC
- qemu_set_cloexec(fd);
+ qemu_set_cloexec(*fd);
#endif
+ }
+
+ return true;
+}
+
+static void qio_channel_cleanup_fds(int **fds, size_t *nfds)
+{
+ for (size_t i = 0; i < *nfds; i++) {
+ if ((*fds)[i] < 0) {
+ continue;
}
- *nfds += gotfds;
+ close((*fds)[i]);
}
+
+ g_clear_pointer(fds, g_free);
+ *nfds = 0;
}
@@ -559,9 +587,16 @@ static ssize_t qio_channel_socket_readv(QIOChannel *ioc,
}
if (fds && nfds) {
- qio_channel_socket_copy_fds(
- &msg, fds, nfds,
- flags & QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING);
+ bool preserve_blocking =
+ flags & QIO_CHANNEL_READ_FLAG_FD_PRESERVE_BLOCKING;
+
+ qio_channel_socket_copy_fds(&msg, fds, nfds);
+
+ if (!qio_channel_handle_fds(*fds, *nfds,
+ preserve_blocking, errp)) {
+ qio_channel_cleanup_fds(fds, nfds);
+ return -1;
+ }
}
return ret;
--
2.48.1