Hi,
On Wed, Jun 19, 2019 at 04:56:41PM +0100, Frediano Ziglio wrote:
> Avoid having to destroy and create a new GSource every time
> we change event mask.
> Interfaces required for this patch are available since GLib 2.36.
> on Windows GPollFD::fd can be an HANDLE but not a socket.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/event-loop.c | 97 +++++++++++++++++++++++++++++++++++++++------
> 1 file changed, 85 insertions(+), 12 deletions(-)
>
> diff --git a/server/event-loop.c b/server/event-loop.c
> index 80af2954f..33db4ffb0 100644
> --- a/server/event-loop.c
> +++ b/server/event-loop.c
> @@ -85,14 +85,6 @@ static void timer_remove(const SpiceCoreInterfaceInternal *iface,
> g_free(timer);
> }
>
> -struct SpiceWatch {
> - GMainContext *context;
> - void *opaque;
> - GSource *source;
> - GIOChannel *channel;
> - SpiceWatchFunc func;
> -};
> -
> static GIOCondition spice_event_to_giocondition(int event_mask)
> {
> GIOCondition condition = 0;
> @@ -117,6 +109,15 @@ static int giocondition_to_spice_event(GIOCondition condition)
> return event;
> }
>
> +#ifdef _WIN32
> +struct SpiceWatch {
> + GMainContext *context;
> + void *opaque;
> + GSource *source;
> + GIOChannel *channel;
> + SpiceWatchFunc func;
> +};
> +
> static gboolean watch_func(GIOChannel *source, GIOCondition condition,
> gpointer data)
> {
> @@ -161,11 +162,7 @@ static SpiceWatch *watch_add(const SpiceCoreInterfaceInternal *iface,
>
> watch = g_new0(SpiceWatch, 1);
> watch->context = iface->main_context;
> -#ifndef _WIN32
> - watch->channel = g_io_channel_unix_new(fd);
> -#else
> watch->channel = g_io_channel_win32_new_socket(fd);
> -#endif
> watch->func = func;
> watch->opaque = opaque;
>
> @@ -184,6 +181,82 @@ static void watch_remove(const SpiceCoreInterfaceInternal *iface,
> g_free(watch);
> }
>
> +#else
> +
> +struct SpiceWatch {
> + GSource source;
> + GPollFD pollfd;
> +};
> +
> +static gboolean
> +spice_watch_check(GSource *source)
> +{
> + SpiceWatch *watch = SPICE_CONTAINEROF(source, SpiceWatch, source);
> +
> + return watch->pollfd.events & watch->pollfd.revents;
> +}
> +
> +static gboolean
> +spice_watch_dispatch(GSource *source,
> + GSourceFunc callback,
> + gpointer user_data)
> +{
> + SpiceWatch *watch = SPICE_CONTAINEROF(source, SpiceWatch, source);
> + SpiceWatchFunc func = (SpiceWatchFunc)(void*) callback;
> +
> + func(watch->pollfd.fd, giocondition_to_spice_event(watch->pollfd.revents), user_data);
> + /* timer might be free after func(), don't touch */
> +
> + return G_SOURCE_CONTINUE;
> +}
> +
> +static GSourceFuncs spice_watch_funcs = {
> + .check = spice_watch_check,
> + .dispatch = spice_watch_dispatch,
> +};
> +
> +static void watch_update_mask(const SpiceCoreInterfaceInternal *iface,
> + SpiceWatch *watch, int event_mask)
> +{
> + GIOCondition old_condition = watch->pollfd.events;
> + GIOCondition new_condition = spice_event_to_giocondition(event_mask);
> +
> + watch->pollfd.events = new_condition;
> + if (old_condition && !new_condition) {
> + g_source_remove_poll(&watch->source, &watch->pollfd);
> + } else if (!old_condition && new_condition) {
> + g_source_add_poll(&watch->source, &watch->pollfd);
There is a note in the manual "Newly-written event sources should
try to use g_source_add_unix_fd() instead of this API." which is
present since 2.36.
> + }
> +}
> +
> +static SpiceWatch *watch_add(const SpiceCoreInterfaceInternal *iface,
> + int fd, int event_mask, SpiceWatchFunc func, void *opaque)
> +{
> + SpiceWatch *watch = (SpiceWatch *) g_source_new(&spice_watch_funcs, sizeof(SpiceWatch));
> +
> + spice_return_val_if_fail(fd != -1, NULL);
> + spice_return_val_if_fail(func != NULL, NULL);
If an issue in the guards, you would be leaking the GSource
> +
> + watch->pollfd.fd = fd;
> + watch->pollfd.events = 0;
> +
> + g_source_set_callback(&watch->source, (GSourceFunc)(void*)(SpiceWatchFunc) func, opaque, NULL);
I guess G_SOURCE_FUNC should work for casting *but* that is in
2.58 only :(
Potential addition to glib-compat.h ?
> +
> + g_source_attach(&watch->source, iface->main_context);
> +
> + watch_update_mask(iface, watch, event_mask);
> +
> + return watch;
> +}
> +
> +static void watch_remove(const SpiceCoreInterfaceInternal *iface,
> + SpiceWatch *watch)
> +{
Not sure on situations that watch_remove() can be called actually
but you might considering add a conditional with
g_source_is_destroyed() to avoid unref multiple times. Perhaps
not important.
Cheers,
Victor
> + g_source_destroy(&watch->source);
> + g_source_unref(&watch->source);
> +}
> +#endif
> +
> const SpiceCoreInterfaceInternal event_loop_core = {
> .timer_add = timer_add,
> .timer_start = timer_start,
> --
> 2.20.1
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel