>
> 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
>
>
Sounds good to me.
Should I send another version or just put this sentence on
the comment above the union?
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel