[Spice-devel] [PATCH 06/19] Convert Dispatcher and MainDispatcher to GObjects

[Jonathon Jongsma]

On Fri, 2016-02-19 at 06:54 -0500, Frediano Ziglio wrote:
> > 
> > From: Jonathon Jongsma <jjongsma at redhat.com>
> > 
> > 

> > -struct MainDispatcher {
> > -    Dispatcher base;
> > -    SpiceCoreInterfaceInternal *core;
> > -    RedsState *reds;
> > +G_DEFINE_TYPE(MainDispatcher, main_dispatcher, TYPE_DISPATCHER)
> > +
> > +#define MAIN_DISPATCHER_PRIVATE(o) (G_TYPE_INSTANCE_GET_PRIVATE((o),
> > TYPE_MAIN_DISPATCHER, MainDispatcherPrivate))
> > +
> > +struct MainDispatcherPrivate
> > +{
> > +    SpiceCoreInterfaceInternal *core; /* weak */
> > +    RedsState *reds; /* weak */
> 
> Not that weak, try to set them NULL and see what's happening...
> 


Yes, perhaps a poor choice of words. My intention was to say that the
MainDispatcher does not own these objects, and therefore does not have to free
them in the destructor. If they were GObjects, we would probably take a
reference, but they're not.



> > diff --git a/server/red-dispatcher.c b/server/red-dispatcher.c
> > index 33cd12f..4b839a9 100644
> > --- a/server/red-dispatcher.c
> > +++ b/server/red-dispatcher.c
> > @@ -48,7 +48,7 @@ struct AsyncCommand {
> >  struct RedDispatcher {
> >      QXLWorker base;
> >      QXLInstance *qxl;
> > -    Dispatcher dispatcher;
> > +    Dispatcher *dispatcher;
> 
> Here you are changing inheritance to association... (1)

Yeah, sort of. But I did it for a couple of reasons: 
- RedDispatcher already "inherits" QXLWorker, and inheritance of this sort only
really works if the parent type is the first member within the struct
- Most of the internals of the Dispatcher struct were moved to an internal
private struct anyway, so embedding the struct here doesn't really gain you
much. You can't access that private data directly.

So in my mind the only real difference is how the memory is allocated: either as
part of the RedDispatcher, or separately in the red_dispatcher_init() function.
Since there's only a very small number of dispatcher objects and they live the
entire life of the the application, that doesn't strike me as a very big deal. 

Do you object to this change?


Jonathon