> On Fri, 2015-10-23 at 10:29 -0400, Frediano Ziglio wrote:
>
> > >
> > > +#define RED_CHANNEL(Channel) ((RedChannel *)(Channel))
> > > +
> >
> > I really don't like these kind of macros, they are really type
> > unsafe,
> > what about
> >
> > void *p = malloc(50);
> >
> > red_channel_client_disconnect(RED_CHANNEL(p));
> >
> > the more explicit
> >
> > void *p = malloc(50);
> >
> > red_channel_client_disconnect((RedChannel*) p);
> >
> > make it more explicit that we are doing something dangerous.
> >
> > 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)
> >
>
>
> But this just sets things up for the future conversion to GObject. When
> RedChannel is derived from GObject, RED_CHANNEL() will be the standard
> way to cast in a relatively typesafe manner.
>
> Jonathon
>
Fine, but is like posting two patches in a set where first patch introduce
a feature and the second fix it. Usually maintainer will ask to squash them.
The patches are quite far apart perhaps a comment would be enough.
It will help a future maintainer of the code which will look at the patch.
Note that I didn't propose to remove the patch but define in a more safe way.
I honestly prefer program code where a change cause a compile error to code
where small changes cause unexpected and hard to fix bugs.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel