Re: [PATCH 09/14] server: make cursor channel private

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> 
> On Mon, Oct 26, 2015 at 06:48:38AM -0400, Frediano Ziglio wrote:
> > 
> > > 
> > > On Fri, Oct 23, 2015 at 10:29:04AM -0400, Frediano Ziglio wrote:
> > > > It would be much better to define a public structure like
> > > > 
> > > > struct CursorChannel {
> > > >    CommonChannel common;
> > > > };
> > > > 
> > > > and a private one (in cursor-channel.c) like
> > > > 
> > > > struct CursorChannel {
> > > >    CommonChannel common;
> > > >    ... any field needed ...
> > > > }
> > > > 
> > > > and define the macro as
> > > > 
> > > > #define RED_CHANNEL(Channel) (&(Channel)->common.base)
> > > 
> > > This kind of macros does not work as soon as you introduce another level
> > > of inheritance:
> > > 
> > > struct SpecializedCursorChannel {
> > >     CursorChannel parent;
> > >     /* more stuff */
> > > };
> > > 
> > > You cannot use your RED_CHANNEL macro to get a RedChannel from a
> > > SpecializedCursorChannel instance.
> > > 
> > > Christophe
> > > 
> > 
> > Yes, this does not prevent the macro to convert an integer array to a
> > RedChannel
> > without warnings. How was implemented before prevented it.
> 
> I understand what benefits your suggested implementation had, but my
> point is that it will not handle some perfectly valid use cases.
> As Jonathon was pointing out, this can be seen as a step on the way
> toward a more object-oriented approach.
> 
> At this point, the gist of the patch is using the macro rather than
> direct foo->base access, so how it's implemented is not that important
> (ie we can go with your suggestion if that works everywhere). It's
> likely to be replaced with a glorified cast (cast + runtime type check)
> in the future though.
> 
> Christophe
> 

Hi,
  just to make it clear. You and Jonathon agreed this patch and with macros
as is just temporary and as it's the usual way the "class" are implemented in
C. However the patch is still in the patchset for two reasons:
- there are other patches this patch depends on. It's not easy to move
  the patch not not straighforward;
- nobody acked it!

If somebody ack it I could spend some time and rebase/push it.

Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]