Re: [PATCH 08/12] util/oslib-win32: Revert warning on WSAEventSelect() failure

["Daniel P. Berrangé" <berrange@redhat.com>]

On Fri, Aug 08, 2025 at 10:08:19AM +0200, 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 specifies that errors are to be ignored.  If warnings are
> wanted, the caller must pass &error_warn.
> 
> I'm not familiar with the calling code, so I can't say whether it will
> work after WSAEventSelect() failure.  If it doesn't, then this should
> be an error.  If it does, then why bother the user with a warning that
> isn't actionable, and likely confusing?
> 
> The warning goes back to commit f5fd677ae7cf (win32/socket: introduce
> qemu_socket_select() helper).  Before that commit, the error was
> ignored, as indicated by passing a null @errp.  Revert to that
> behavior.
> 
> Cc: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> ---
>  util/oslib-win32.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/util/oslib-win32.c b/util/oslib-win32.c
> index b7351634ec..136a8fe118 100644
> --- a/util/oslib-win32.c
> +++ b/util/oslib-win32.c
> @@ -296,10 +296,6 @@ bool qemu_socket_select(int sockfd, WSAEVENT hEventObject,
>  {
>      SOCKET s = _get_osfhandle(sockfd);
>  
> -    if (errp == NULL) {
> -        errp = &error_warn;
> -    }

This makes sense, but I'd want the callers to be using warn_report
instead. Ideally some (but not all) of the callers would propagate
the error, but this isn't practical with the QIOChannel create
watch function usage. I'd want to keep Error *errp on this function
though, and have warn_report as a sign to our future selves that
this is still not ideal.

> -
>      if (s == INVALID_SOCKET) {
>          error_setg(errp, "invalid socket fd=%d", sockfd);
>          return false;
> -- 
> 2.49.0
> 
> 

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|