[Spice-devel] [PATCH v2 2/3] tests: create and use a template file for events

[Frediano Ziglio]

> On Fri, Dec 18, 2015 at 09:11:04AM -0500, Frediano Ziglio wrote:
> > > 
> > > On Fri, Dec 18, 2015 at 06:41:57AM -0500, Frediano Ziglio wrote:
> > > > The patch is this anyway
> > > > http://cgit.freedesktop.org/~fziglio/spice-server/commit/?id=452edd8f7aa25fc1e69b6c2a747f59f58ab07f32
> > > > 
> > > > So could be your patch can work.  However the main idea of my
> > > > patch was not to fix this "strange" code but to have a single
> > > > source for the event loop with better (and automated) way of
> > > > testing it.
> > > 
> > > Which "strange" code are you referring to ? The one from this cgit
> > > link, or the one modified by my patch?  I don't think assuming that
> > > 'opaque' will always be a RedWorker instance is a good assumption to
> > > make, so this is a NACK from me for this patch as it is now.
> > > 
> > 
> > NACK what? The code is in master!
> 
> NACK to the patch I'm replying to ("tests: create and use a template
> file for events"), and to the next one which depends on this one, which
> are not in master as far as I can tell. I don't think there is code in
> master assuming that the 'opaque' data passed to timer callbacks is
> actually a RedWorker instance.
>

I checked the code with patches and so on.
You are right, this assumption is just for watches, not for timers.
Actually was not there even for timers on original Glib patch.
Looks like was introduced after some changes.
 
> > > > And still I would prefer a simple __thread variable.
> > > 
> > > I prefer the more explicit get_main_context() calls as it's obvious
> > > something is going on. With __thread, you had to dig up to the
> > > declaration of the variable to realize there is some magic involved.
> > > 
> > > Christophe
> > > 
> > 
> > You can use an inline function if you prefer.
> > 
> > Sometimes looks like Glib is the bible...
> 
> Not the bible, just that it has a convenience function doing exactly
> what we need so... As things stand, a simple helper + __thread will
> indeed do the trick, if we were to start using gio for asynchronous IO,
> g_main_context_push_thread_default() would become much more useful.
> 
> Christophe
> 

Although I think the right solution is not differentiate on thread but
on core (actually not that easy) I think the thread solution (having
different main_context for different thread using either __thread,
g_main_context_get_thread_default or similar) is one of the better
work-around.

I'll try to add an argument to the SpiceCoreInterface functions
(without changing ABI). Don't know how hard is it.

Frediano