[PATCH v3 08/13] util/oslib-win32: Do not treat null @errp as &error_warn

Markus Armbruster posted 13 patches 5 days, 7 hours ago
Maintainers: Jonathan Cameron <jonathan.cameron@huawei.com>, Fan Ni <fan.ni@samsung.com>, "Alex Bennée" <alex.bennee@linaro.org>, Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>, Dmitry Osipenko <dmitry.osipenko@collabora.com>, "Michael S. Tsirkin" <mst@redhat.com>, Gustavo Romero <gustavo.romero@linaro.org>, Jason Wang <jasowang@redhat.com>, Elena Ufimtseva <elena.ufimtseva@oracle.com>, Jagannathan Raman <jag.raman@oracle.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Markus Armbruster <armbru@redhat.com>, Michael Roth <michael.roth@amd.com>, Stefan Weil <sw@weilnetz.de>, "Daniel P. Berrangé" <berrange@redhat.com>, Steve Sistare <steven.sistare@oracle.com>, Peter Xu <peterx@redhat.com>, Fabiano Rosas <farosas@suse.de>, "Dr. David Alan Gilbert" <dave@treblig.org>, Samuel Thibault <samuel.thibault@ens-lyon.org>, Richard Henderson <richard.henderson@linaro.org>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, Stefan Hajnoczi <stefanha@redhat.com>, Fam Zheng <fam@euphon.net>
[PATCH v3 08/13] util/oslib-win32: Do not treat null @errp as &error_warn
Posted by Markus Armbruster 5 days, 7 hours ago
qemu_socket_select() and its wrapper qemu_socket_unselect() treat a
null @errp as &error_warn.  This is wildly inappropriate.  A caller
passing null @errp specifies that errors are to be ignored.  If
warnings are wanted, the caller must pass &error_warn.

Change callers to do that, and drop the inappropriate treatment of
null @errp.

This assumes that warnings are wanted.  I'm not familiar with the
calling code, so I can't say whether it will work when the socket is
invalid, or WSAEventSelect() fails.  If it doesn't, then this should
be an error instead of a warning.  Invalid socket might even be a
programming error.

These warnings were introduced in commit f5fd677ae7cf (win32/socket:
introduce qemu_socket_select() helper).  I considered reverting to
silence, but Daniel Berrangé asked for the warnings to be preserved.

Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
---
 io/channel-socket.c | 4 ++--
 io/channel-watch.c  | 2 +-
 util/aio-win32.c    | 2 +-
 util/oslib-win32.c  | 6 +-----
 4 files changed, 5 insertions(+), 9 deletions(-)

diff --git a/io/channel-socket.c b/io/channel-socket.c
index e53d9ac76f..fdc7670867 100644
--- a/io/channel-socket.c
+++ b/io/channel-socket.c
@@ -454,7 +454,7 @@ static void qio_channel_socket_finalize(Object *obj)
             }
         }
 #ifdef WIN32
-        qemu_socket_unselect(ioc->fd, NULL);
+        qemu_socket_unselect(ioc->fd, &error_warn);
 #endif
         close(ioc->fd);
         ioc->fd = -1;
@@ -929,7 +929,7 @@ qio_channel_socket_close(QIOChannel *ioc,
 
     if (sioc->fd != -1) {
 #ifdef WIN32
-        qemu_socket_unselect(sioc->fd, NULL);
+        qemu_socket_unselect(sioc->fd, &error_warn);
 #endif
         if (qio_channel_has_feature(ioc, QIO_CHANNEL_FEATURE_LISTEN)) {
             socket_listen_cleanup(sioc->fd, errp);
diff --git a/io/channel-watch.c b/io/channel-watch.c
index 64b486e378..ec76bd1ec6 100644
--- a/io/channel-watch.c
+++ b/io/channel-watch.c
@@ -283,7 +283,7 @@ GSource *qio_channel_create_socket_watch(QIOChannel *ioc,
 
     qemu_socket_select(sockfd, ioc->event,
                        FD_READ | FD_ACCEPT | FD_CLOSE |
-                       FD_CONNECT | FD_WRITE | FD_OOB, NULL);
+                       FD_CONNECT | FD_WRITE | FD_OOB, &error_warn);
 
     source = g_source_new(&qio_channel_socket_source_funcs,
                           sizeof(QIOChannelSocketSource));
diff --git a/util/aio-win32.c b/util/aio-win32.c
index 6583d5c5f3..b125924433 100644
--- a/util/aio-win32.c
+++ b/util/aio-win32.c
@@ -121,7 +121,7 @@ void aio_set_fd_handler(AioContext *ctx,
 
         QLIST_INSERT_HEAD_RCU(&ctx->aio_handlers, node, node);
         event = event_notifier_get_handle(&ctx->notifier);
-        qemu_socket_select(fd, event, bitmask, NULL);
+        qemu_socket_select(fd, event, bitmask, &error_warn);
     }
     if (old_node) {
         aio_remove_fd_handler(ctx, old_node);
diff --git a/util/oslib-win32.c b/util/oslib-win32.c
index b9ce2f96ee..6a2367c89d 100644
--- a/util/oslib-win32.c
+++ b/util/oslib-win32.c
@@ -182,7 +182,7 @@ bool qemu_set_blocking(int fd, bool block, Error **errp)
     unsigned long opt = block ? 0 : 1;
 
     if (block) {
-        qemu_socket_unselect(fd, NULL);
+        qemu_socket_unselect(fd, &error_warn);
     }
 
     if (ioctlsocket(fd, FIONBIO, &opt) != NO_ERROR) {
@@ -293,10 +293,6 @@ bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
 {
     SOCKET s = _get_osfhandle(sockfd);
 
-    if (errp == NULL) {
-        errp = &error_warn;
-    }
-
     if (s == INVALID_SOCKET) {
         error_setg(errp, "invalid socket fd=%d", sockfd);
         return false;
-- 
2.49.0


Re: [PATCH v3 08/13] util/oslib-win32: Do not treat null @errp as &error_warn
Posted by Philippe Mathieu-Daudé 5 days, 7 hours ago
On 23/9/25 11:09, Markus Armbruster wrote:
> qemu_socket_select() and its wrapper qemu_socket_unselect() treat a
> null @errp as &error_warn.  This is wildly inappropriate.  A caller
> passing null @errp specifies that errors are to be ignored.  If
> warnings are wanted, the caller must pass &error_warn.
> 
> Change callers to do that, and drop the inappropriate treatment of
> null @errp.
> 
> This assumes that warnings are wanted.  I'm not familiar with the
> calling code, so I can't say whether it will work when the socket is
> invalid, or WSAEventSelect() fails.  If it doesn't, then this should
> be an error instead of a warning.  Invalid socket might even be a
> programming error.
> 
> These warnings were introduced in commit f5fd677ae7cf (win32/socket:
> introduce qemu_socket_select() helper).  I considered reverting to
> silence, but Daniel Berrangé asked for the warnings to be preserved.
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Reviewed-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>   io/channel-socket.c | 4 ++--
>   io/channel-watch.c  | 2 +-
>   util/aio-win32.c    | 2 +-
>   util/oslib-win32.c  | 6 +-----
>   4 files changed, 5 insertions(+), 9 deletions(-)

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>