> On Wed, Mar 23, 2016 at 12:48:27PM +0000, Frediano Ziglio wrote:
> > From: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> >
> > 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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel