[Spice-devel] [PATCH 01/13] Convert Dispatcher and MainDispatcher to GObjects

[Frediano Ziglio]

> On Wed, Mar 23, 2016 at 12:48:27PM +0000, Frediano Ziglio wrote:
> > From: Jonathon Jongsma <jjongsma at redhat.com>
> > 
> > Allows more explicit inheritance relationship, and numerous other
> > advantages.
> > ---
> >  server/dispatcher.c      | 234
> >  ++++++++++++++++++++++++++++++++++++-----------
> >  server/dispatcher.h      |  53 ++++++-----
> >  server/main-dispatcher.c | 157 +++++++++++++++++++++++++------
> >  server/main-dispatcher.h |  25 +++++
> >  server/red-qxl.c         |  78 ++++++++--------
> >  5 files changed, 405 insertions(+), 142 deletions(-)
> > 
> > diff --git a/server/dispatcher.c b/server/dispatcher.c
> > index d6c03ca..cd0b4ee 100644
> > --- a/server/dispatcher.c
> > +++ b/server/dispatcher.c
> > @@ -1,6 +1,5 @@
> > -/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */
> >  /*
> > -   Copyright (C) 2009-2012 Red Hat, Inc.
> > +   Copyright (C) 2009-2016 Red Hat, Inc.
> >  
> >     This library is free software; you can redistribute it and/or
> >     modify it under the terms of the GNU Lesser General Public
> > @@ -39,6 +38,156 @@
> >  #include <signal.h>
> >  #endif
> >  
> > +G_DEFINE_TYPE(Dispatcher, dispatcher, G_TYPE_OBJECT)
> > +
> > +#define DISPATCHER_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE ((o),
> > TYPE_DISPATCHER, DispatcherPrivate))
> > +
> > +struct DispatcherPrivate {
> > +    int recv_fd;
> > +    int send_fd;
> > +    pthread_t self;
> > +    pthread_mutex_t lock;
> > +    DispatcherMessage *messages;
> > +    int stage;  /* message parser stage - sender has no stages */
> > +    size_t max_message_type;
> 
> A bit odd that this is size_t here and in the _new function, but the
> property is only a guint. Apart from this, looks good to me (but
> red_qxl_get_dispatcher() + the changes in red_worker_new() make me
> wonder if 'dispatcher' should be some kind of interface implemented by
> RedQXL rather than something embedded in RedQXL.
> 
> Christophe
> 

max_message_type is compared with message types which are uint32_t and used
to limit an index to an array where the index is a message type so yes, guint
would be better (I'll change).

About dispatcher as an interface that would require defining a dispatcher
interface (as currently is a class) and I don't see the point in implementing
the dispatcher interface twice (one for maindispatcher and one for RedQXL).
QXL object run in a separate worker (RedWorker) and the main interface talk
with that worker using a Dispatcher. I don't see the reason why RedQXL should
provide an interface to a dispatcher.

Frediano