[Spice-devel] [PATCH 1/5] rename red_dispatcher_ functions to red_qxl_

[Frediano Ziglio]

> 
> Hey,
> 
> On Mon, Feb 29, 2016 at 01:34:32PM -0600, Jonathon Jongsma wrote:
> > Frediano and I talked about this last week and basically agreed on renaming
> > RedDispatcher to RedQXL. A little more justification for the rename:
> > 
> > There are already a couple other types with Dispatcher in their names:
> > Dispatcher and its sub-class MainDispatcher. These classes, as the name
> > implies,
> > basically dispatch messages between different threads. The name
> > RedDispatcher
> > implies that this type is related to the other Dispatcher classes, but
> > that's
> > not really true.
> 
> I definitely agree with that, actually I worked on some similar rename a
> while back, but never finished it :(
> 
> > It does *own* a Dispatcher to enable it to talk to RedWorker,
> > but the class itself is really an implementation of QXLInstance.
> 
> Hmm, I agree it's an implementation of QXLInstance, but is also
> apparently a child class of QXLWorker as
> 
> struct RedDispatcher {
>     QXLWorker base;
>     ...
> }
> seems to imply. The old patches I have were trying to merge
> QXLWorker and RedDispatcher through a little bit of juggling at
> allocation time
> 

I have a patch to change even this. Consider that QXLWorker is the
old pure interface but all methods are marked as deprecated.
Technical speaking QXL has to implement QXLWorker interface for
compatibility (in OO is a different relationship).

> struct QXLWorkerInternal {
>     QXLWorker base;
>     QXLWorkerPrivate *private;
> }
> 
> + some get_private(QXLWorker *worker) helper.
> 
> I'm not saying this is better than the patches which are proposed here,
> just trying to discuss how to make all of this clearer :)
> 
> Also, QXLInstance is
> struct QXLInstance {
>     SpiceBaseInstance  base;
>     int                id;
>     QXLState           *st;
> };
> 
> with QXLState an internal structure as well (defined in reds.h), and
> RedDispatcher/RedQXL is one of the few members of this QXLState
> structure. Should they be merged?
> 

I think you should check patch 4/5 :)

> > So I think the
> > RedDispatcher name introduces confusion and makes it harder to understand
> > the
> > structure of the code. I think that justifies a rename.
> > 
> > Is everybody else OK with the rename?
> 
> I'm ok with _a_ rename of RedDispatcher, just wondering whether more
> clarifications are needed/if we can avoid introducing yet another name
> with QXL in it.
> 
> Christophe
> 

One reason for the name was that 100% of spice_qxl_ functions have an
implementation in red-dispatcher.c (not taking into account the qxl_worker
functions).

Frediano