[Spice-devel] [PATCH 09/14] server: make cursor channel private

[Christophe Fergeau]

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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/spice-devel/attachments/20151026/ba53f7b6/attachment.sig>