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@xxxxxxxxxx> > > > --- > > > 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel