[Spice-devel] [PATCH spice-server] Remove core_public and core_interface_adapter globals usage

[Jonathon Jongsma]

On Tue, 2016-11-15 at 04:53 -0500, Frediano Ziglio wrote:
> > 
> > 
> > On Fri, 2016-11-11 at 12:08 +0000, Frediano Ziglio wrote:
> > > 
> > > Avoid not constant globals.
> > > 
> > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > ---
> > >  server/dummy-channel.c          |  5 ++-
> > >  server/event-loop.c             | 23 ++++++-----
> > >  server/red-channel-client.c     | 32 +++++++--------
> > >  server/red-common.h             | 17 ++++----
> > >  server/reds-private.h           |  2 +-
> > >  server/reds.c                   | 88 ++++++++++++++++++---------
> > > ----
> > > ----------
> > >  server/tests/basic_event_loop.c | 37 ++++++++++++++---
> > >  7 files changed, 114 insertions(+), 90 deletions(-)
> > > 
> > > diff --git a/server/dummy-channel.c b/server/dummy-channel.c
> > > index f13c8f8..aecd3a6 100644
> > > --- a/server/dummy-channel.c
> > > +++ b/server/dummy-channel.c
> ....
> > 
> > > 
> > >  
> > > -    void (*channel_event)(int event, SpiceChannelEventInfo
> > > *info);
> > > +    void (*channel_event)(const SpiceCoreInterfaceInternal
> > > *iface,
> > > int event, SpiceChannelEventInfo *info);
> > >  
> > > -    GMainContext *main_context;
> > > +    union {
> > > +        GMainContext *main_context;
> > > +        SpiceCoreInterface *public_interface;
> > > +    };
> > >  };
> > 
> > The last time I reviewed this patch (in June!), I ACKed it, but I
> > asked
> > for a comment here explaining why the main_context and
> > public_interface
> > were mutually exclusive. Now that it's the future, my prediction
> > that I
> > might find it confusing in the future has come true. I had to
> > convince
> > myself again. So I'd like to repeat the request for a comment
> > again.
> > 
> > Otherwise it looks fine.
> > 
> > Jonathon
> > 
> 
> I surely missed.
> 
> What about:
> 
> /* Implementation data.
>  * Usually these fields are is implemented subclassing the object and
>  * adding new fields, in this case the choices are limited and both
>  * pointers so use an union to store all possible values.
>  */
> 
> Frediano

hmm. That doesn't quite do it for me.  Let me try:

This structure is an adapter that allows us to use the same API to
implement the core interface in a couple different ways. The first
method is to use a public SpiceCoreInterface provided to us by the
library user (for example, qemu). The second method is to implement the
core interface functions using the glib event loop. In order to avoid
global variables, each method needs to store additional data in this
adapter structure. Instead of using a generic void* data parameter, we
provide a bit more type-safety by using a union to store the type of
data needed by each implementation.

Is that an accurate description?

Jonathon