On 18/12/20 14:57, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> Move the declaration of the function to a windows-specific header.
>
> The only user left now is SLIRP, which needs special treatement since
> it doesn't provide event objects itself for the socket/fds.
>
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This patch is not needed to fix the build, right? (I don't disagree
with it, but I wanted to understand why _you_ thought it was an
improvement).
My point is that this is corenercase-ish and specific to Windows, I don't really think it's worth it to be in main-loop.h, with stubs to take care of the non-windows case (and possibly error prone, if you link against it by mistake).
If we were to be using that call consistently in every part of qemu using an fd and the main-loop, it would make sense to keep it "generic". But it's not being used that way.
Paolo
> ---
> include/qemu/main-loop.h | 2 --
> include/sysemu/os-win32.h | 2 ++
> net/slirp.c | 2 ++
> util/main-loop.c | 4 ----
> 4 files changed, 4 insertions(+), 6 deletions(-)
>
> diff --git a/include/qemu/main-loop.h b/include/qemu/main-loop.h
> index d6892fd208..bf93fd691d 100644
> --- a/include/qemu/main-loop.h
> +++ b/include/qemu/main-loop.h
> @@ -310,8 +310,6 @@ void qemu_cond_timedwait_iothread(QemuCond *cond, int ms);
>
> /* internal interfaces */
>
> -void qemu_fd_register(int fd);
> -
> QEMUBH *qemu_bh_new(QEMUBHFunc *cb, void *opaque);
> void qemu_bh_schedule_idle(QEMUBH *bh);
>
> diff --git a/include/sysemu/os-win32.h b/include/sysemu/os-win32.h
> index 5346d51e89..aa462e3ef6 100644
> --- a/include/sysemu/os-win32.h
> +++ b/include/sysemu/os-win32.h
> @@ -117,6 +117,8 @@ static inline void qemu_funlockfile(FILE *f)
> {
> }
>
> +void qemu_fd_register(int fd);
> +
> /* We wrap all the sockets functions so that we can
> * set errno based on WSAGetLastError()
> */
> diff --git a/net/slirp.c b/net/slirp.c
> index 77042e6df7..b54c2882dc 100644
> --- a/net/slirp.c
> +++ b/net/slirp.c
> @@ -196,7 +196,9 @@ static void net_slirp_timer_mod(void *timer, int64_t expire_timer,
>
> static void net_slirp_register_poll_fd(int fd, void *opaque)
> {
> +#ifdef WIN32
> qemu_fd_register(fd);
> +#endif
> }
>
> static void net_slirp_unregister_poll_fd(int fd, void *opaque)
> diff --git a/util/main-loop.c b/util/main-loop.c
> index 6470f8eae3..744b42fc54 100644
> --- a/util/main-loop.c
> +++ b/util/main-loop.c
> @@ -179,10 +179,6 @@ static int max_priority;
> static int glib_pollfds_idx;
> static int glib_n_poll_fds;
>
> -void qemu_fd_register(int fd)
> -{
> -}
> -
> static void glib_pollfds_fill(int64_t *cur_timeout)
> {
> GMainContext *context = g_main_context_default();
>