[Spice-devel] [PATCH 23/24] server: move dispatcher GSource handling code

[Frediano Ziglio]

> 
> From: Marc-André Lureau <marcandre.lureau at gmail.com>
> 
> ---
>  server/dispatcher.c | 25 +++++++++++++++++++++++++
>  server/dispatcher.h |  2 ++
>  server/red_worker.c | 40 ++++++++++++----------------------------
>  3 files changed, 39 insertions(+), 28 deletions(-)
> 
> diff --git a/server/dispatcher.c b/server/dispatcher.c
> index d6c03ca..974fa75 100644
> --- a/server/dispatcher.c
> +++ b/server/dispatcher.c
> @@ -289,3 +289,28 @@ int dispatcher_get_recv_fd(Dispatcher *dispatcher)
>  {
>      return dispatcher->recv_fd;
>  }
> +
> +static gboolean dispatch_cb(GIOChannel *source, GIOCondition condition,
> +                            gpointer data)
> +{
> +    Dispatcher *dispatcher = data;
> +
> +    spice_debug(NULL);
> +    dispatcher_handle_recv_read(dispatcher);
> +
> +    /* FIXME: remove source cb if error */
> +    return TRUE;
> +}
> +
> +void dispatcher_attach(Dispatcher *dispatcher, GMainContext *main_context)
> +{
> +    spice_return_if_fail(dispatcher != NULL);
> +    spice_return_if_fail(main_context != NULL);
> +
> +    GIOChannel *channel = g_io_channel_unix_new(dispatcher->recv_fd);
> +    GSource *source = g_io_create_watch(channel, G_IO_IN);
> +
> +    g_source_set_callback(source, (GSourceFunc)dispatch_cb, dispatcher,
> NULL);
> +    g_source_attach(source, main_context);
> +    g_source_unref(source);
> +}
> diff --git a/server/dispatcher.h b/server/dispatcher.h
> index 0d3175f..36ac9ec 100644
> --- a/server/dispatcher.h
> +++ b/server/dispatcher.h
> @@ -72,6 +72,8 @@ void dispatcher_send_message(Dispatcher *dispatcher,
> uint32_t message_type,
>  void dispatcher_init(Dispatcher *dispatcher, size_t max_message_type,
>                       void *opaque);
>  
> +void dispatcher_attach(Dispatcher *dispatcher, GMainContext *main_context);
> +
>  enum {
>      DISPATCHER_NONE = 0,
>      DISPATCHER_ACK,
> diff --git a/server/red_worker.c b/server/red_worker.c
> index 8d1802e..2e3f522 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -1449,11 +1449,12 @@ static void worker_dispatcher_record(void *opaque,
> uint32_t message_type, void *
>      red_record_event(worker->record_fd, 1, message_type,
>      stat_now(worker->clockid));
>  }
>  
> -static void register_callbacks(Dispatcher *dispatcher)
> +static void worker_dispatcher_register(RedWorker *worker, Dispatcher
> *dispatcher)
>  {
> -    dispatcher_register_async_done_callback(
> -                                    dispatcher,
> -                                    worker_handle_dispatcher_async_done);
> +    dispatcher_set_opaque(dispatcher, worker);
> +
> +    dispatcher_register_async_done_callback(dispatcher,
> +
> worker_handle_dispatcher_async_done);

This contains an indentation change (dispatcher_register_async_done_callback call)
and the dispatcher_set_opaque was put inside this function.

>  
>      /* TODO: register cursor & display specific msg in respective channel
>      files */
>      dispatcher_register_handler(dispatcher,
> @@ -1621,21 +1622,11 @@ static void register_callbacks(Dispatcher
> *dispatcher)
>                                  handle_dev_driver_unload,
>                                  sizeof(RedWorkerMessageDriverUnload),
>                                  DISPATCHER_NONE);
> -}
>  
> -
> -
> -static gboolean worker_dispatcher_cb(GIOChannel *source, GIOCondition
> condition,
> -                                     gpointer data)
> -{
> -    RedWorker *worker = data;
> -
> -    spice_debug(NULL);
> -
> dispatcher_handle_recv_read(red_dispatcher_get_dispatcher(worker->red_dispatcher));
> -
> -    return TRUE;
> +    dispatcher_attach(dispatcher, worker->main_context);
>  }
>  
> +
>  typedef struct RedWorkerSource {
>      GSource source;
>      RedWorker *worker;
> @@ -1722,14 +1713,13 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> RedDispatcher *red_dispatcher)
>          }
>      }
>      dispatcher = red_dispatcher_get_dispatcher(red_dispatcher);
> -    dispatcher_set_opaque(dispatcher, worker);
> -
> -    worker->red_dispatcher = red_dispatcher;
> -    worker->qxl = qxl;
> -    register_callbacks(dispatcher);
> +    worker_dispatcher_register(worker, dispatcher);
>      if (worker->record_fd) {
>          dispatcher_register_universal_handler(dispatcher,
>          worker_dispatcher_record);
>      }
> +
> +    worker->red_dispatcher = red_dispatcher;
> +    worker->qxl = qxl;

I still prefer to have these initialize before, the register is
registering callbacks which potentially can access these fields.

>      worker->image_compression = image_compression;
>      worker->jpeg_state = jpeg_state;
>      worker->zlib_glz_state = zlib_glz_state;
> @@ -1742,13 +1732,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> RedDispatcher *red_dispatcher)
>      worker->command_counter = stat_add_counter(worker->stat, "commands",
>      TRUE);
>  #endif
>  
> -    GIOChannel *channel =
> g_io_channel_unix_new(dispatcher_get_recv_fd(dispatcher));
> -    GSource *source = g_io_create_watch(channel, G_IO_IN);
> -    g_source_set_callback(source, (GSourceFunc)worker_dispatcher_cb, worker,
> NULL);
> -    g_source_attach(source, worker->main_context);
> -    g_source_unref(source);
> -
> -    source = g_source_new(&worker_source_funcs, sizeof(RedWorkerSource));
> +    GSource *source = g_source_new(&worker_source_funcs,
> sizeof(RedWorkerSource));
>      RedWorkerSource *wsource = (RedWorkerSource *)source;
>      wsource->worker = worker;
>      g_source_attach(source, worker->main_context);
> --
> 2.4.3
> 

I'll remove style changes

Frediano