>
> On Wed, May 16, 2018 at 3:54 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote:
> >>
> >> On Wed, May 16, 2018 at 10:46:35AM +0200, Christophe Fergeau wrote:
> >> > On Wed, May 16, 2018 at 10:43:20AM +0200, Christophe Fergeau wrote:
> >> > > On Wed, May 16, 2018 at 10:23:01AM +0300, Snir Sheriber wrote:
> >> > > Yes, indeed, this is similar to what you were doing in earlier
> >> > > iterations.
> >> > >
> >> > > > I personally really like the current flow of the request for handle
> >> > > > using the signal and getting it as a response, avoiding of setting
> >> > > > and getting an handle from different components.
> >> > >
> >> > > Using signals as some generic way of calling a getter on some unknown
> >> > > class is rather unusual, and feels like something you should not
> >> > > really
> >> > > be using signals for.
> >> >
> >> > Just realized, this change is most likely an ABI break:
> >>
> >> (I promise I'll stop replying to myself :d). Not sure we are supporting
> >> inheritance from SpiceDisplayChannel, nor that it makes sense, so maybe
> >> not a big problem.
> >>
> >> Christophe
> >>
> >
> > Then class should be declare final is possible and details removed from
> > public interface.
> > Looking also at 941f4adf1322a5d2548af676059003df17613b47 seems to confirm
> > that this class was never meant to be inherited.
>
> That's a commit from 0.1 era ;)
>
Yes, you are right.
I though I looked in the past and found another ABI breakage more recent.
> We may have made some mistakes, but in theory, we bumped
> major/minor/etc correctly enough whenever adding/modifying/removing
> interface (breaking ABI).
>
> During next break (some day), we may use the opportunity to make make
> most channel classes final. This is not a priority though. In the
> meantime, we don't need to add class signal handlers, and simply use
> the private struct for data.
>
>From g_signal_new documentation:
class_offset
The offset of the function pointer in the class structure for this type. Used to invoke a class method generically.
Pass 0 to not associate a class method slot with this signal.
I suggested as a follow up to make this signal "private" not making the documentation
public. I think we don't need to use this signal as a method so I think we can remove
the field in SpiceDisplayChannelClass and pass 0 to avoid ABI break, kind of:
diff --git a/src/channel-display.c b/src/channel-display.c
index 3f2fb3dc..1c0021e8 100644
--- a/src/channel-display.c
+++ b/src/channel-display.c
@@ -470,9 +470,7 @@ static void spice_display_channel_class_init(SpiceDisplayChannelClass *klass)
signals[SPICE_DISPLAY_STREAMING_MODE] =
g_signal_new("streaming-mode",
G_OBJECT_CLASS_TYPE(gobject_class),
- 0,
- G_STRUCT_OFFSET(SpiceDisplayChannelClass,
- streaming_mode),
+ 0, 0,
NULL, NULL,
g_cclosure_user_marshal_POINTER__BOOLEAN,
G_TYPE_POINTER,
diff --git a/src/channel-display.h b/src/channel-display.h
index 9c51aa2a..31293094 100644
--- a/src/channel-display.h
+++ b/src/channel-display.h
@@ -141,8 +141,6 @@ struct _SpiceDisplayChannelClass {
gint x, gint y, gint w, gint h);
void (*display_mark)(SpiceChannel *channel,
gboolean mark);
- void (*streaming_mode)(SpiceChannel *channel,
- gboolean streaming_mode);
/*< private >*/
};
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel