Hey, On Thu, May 17, 2012 at 04:20:12PM +0200, Marc-André Lureau wrote: > The change abc56811de978ad336a651924a21b920cfe677f0 actually added > a field in a public struct while changing overall struct size, the > fields were also reorder, all of this breaks ABI. > > However, we are running out of free space in SpiceChannelClass > struct. And since the SPICE_RESERVED_PADDING was 44 bytes, that is > quite limited on 64bits (only 5 pointers fit). > > I propose we break ABI during this cycle. This means that programs > using spice-gtk will need to be recompiled to use the new library. > (the old library should be parallel installable though). This let us: Yes, makes sense to me > - use a better SPICE_RESERVED_PADDING based on pointer size, since > it is what is usually added for virtual methods > - reset the amount taken from the padding in the various struct > - reorder fields a little > - add some missing "priv" pointers Splitting the priv addition would make this diff more readable ;) > - whatever I am missing that we can still change before next release > > Please comment if I am missing something, or correct me > --- > gtk/Makefile.am | 4 ++-- > gtk/channel-playback.h | 6 +----- > gtk/channel-record.h | 6 +----- > gtk/spice-audio.c | 10 +++++----- > gtk/spice-audio.h | 5 +++-- > gtk/spice-channel.h | 16 ++++++++-------- > gtk/spice-util.h | 2 +- > gtk/usb-device-manager.h | 2 +- > 8 files changed, 22 insertions(+), 29 deletions(-) > > diff --git a/gtk/Makefile.am b/gtk/Makefile.am > index 7b29e61..0327d65 100644 > --- a/gtk/Makefile.am > +++ b/gtk/Makefile.am > @@ -91,7 +91,7 @@ AM_CPPFLAGS = \ > > # http://www.gnu.org/software/libtool/manual/html_node/Updating-version-info.html > SPICE_GTK_LDFLAGS_COMMON = \ > - -version-info 3:0:2 \ > + -version-info 4:0:0 \ > -no-undefined \ > $(VERSION_LDFLAGS) \ > $(NULL) > @@ -158,7 +158,7 @@ nodist_libspice_client_gtkinclude_HEADERS = \ > $(NULL) > > libspice_client_glib_2_0_la_LDFLAGS = \ > - -version-info 7:0:6 \ > + -version-info 8:0:0 \ > -no-undefined \ > $(VERSION_LDFLAGS) \ > $(NULL) > diff --git a/gtk/channel-playback.h b/gtk/channel-playback.h > index 6341cac..9cf68cf 100644 > --- a/gtk/channel-playback.h > +++ b/gtk/channel-playback.h > @@ -65,11 +65,7 @@ struct _SpicePlaybackChannelClass { > void (*playback_stop)(SpicePlaybackChannel *channel); > > /*< private >*/ > - /* > - * If adding fields to this struct, remove corresponding > - * amount of padding to avoid changing overall struct size > - */ > - gchar _spice_reserved[SPICE_RESERVED_PADDING]; > + /* Do not add fields to this struct */ > }; The reason for doing this change is not described in the changelog I think. Are they private structs ? Christophe > > GType spice_playback_channel_get_type(void); > diff --git a/gtk/channel-record.h b/gtk/channel-record.h > index 73bdb76..20a9ad3 100644 > --- a/gtk/channel-record.h > +++ b/gtk/channel-record.h > @@ -65,11 +65,7 @@ struct _SpiceRecordChannelClass { > void (*record_stop)(SpiceRecordChannel *channel); > > /*< private >*/ > - /* > - * If adding fields to this struct, remove corresponding > - * amount of padding to avoid changing overall struct size > - */ > - gchar _spice_reserved[SPICE_RESERVED_PADDING]; > + /* Do not add fields to this struct */ > }; > > GType spice_record_channel_get_type(void); > diff --git a/gtk/spice-audio.c b/gtk/spice-audio.c > index 30c2920..6cf8f01 100644 > --- a/gtk/spice-audio.c > +++ b/gtk/spice-audio.c > @@ -67,7 +67,7 @@ enum { > static void spice_audio_finalize(GObject *gobject) > { > SpiceAudio *self = SPICE_AUDIO(gobject); > - SpiceAudioPrivate *priv = SPICE_AUDIO_GET_PRIVATE(self); > + SpiceAudioPrivate *priv = self->priv; > > if (priv->main_context) { > g_main_context_unref(priv->main_context); > @@ -84,7 +84,7 @@ static void spice_audio_get_property(GObject *gobject, > GParamSpec *pspec) > { > SpiceAudio *self = SPICE_AUDIO(gobject); > - SpiceAudioPrivate *priv = SPICE_AUDIO_GET_PRIVATE(self); > + SpiceAudioPrivate *priv = self->priv; > > switch (prop_id) { > case PROP_SESSION: > @@ -105,7 +105,7 @@ static void spice_audio_set_property(GObject *gobject, > GParamSpec *pspec) > { > SpiceAudio *self = SPICE_AUDIO(gobject); > - SpiceAudioPrivate *priv = SPICE_AUDIO_GET_PRIVATE(self); > + SpiceAudioPrivate *priv = self->priv; > > switch (prop_id) { > case PROP_SESSION: > @@ -152,9 +152,9 @@ static void spice_audio_class_init(SpiceAudioClass *klass) > g_type_class_add_private(klass, sizeof(SpiceAudioPrivate)); > } > > -static void spice_audio_init(SpiceAudio *self G_GNUC_UNUSED) > +static void spice_audio_init(SpiceAudio *self) > { > - /* FIXME: self->priv = SPICE_AUDIO_GET_PRIVATE(audio) when ABI break */ > + self->priv = SPICE_AUDIO_GET_PRIVATE(self); > } > > static void connect_channel(SpiceAudio *self, SpiceChannel *channel) > diff --git a/gtk/spice-audio.h b/gtk/spice-audio.h > index 732fea1..ebc4946 100644 > --- a/gtk/spice-audio.h > +++ b/gtk/spice-audio.h > @@ -52,7 +52,8 @@ typedef struct _SpiceAudioPrivate SpiceAudioPrivate; > */ > struct _SpiceAudio { > GObject parent; > - /* FIXME: break ABI!! SpiceAudioPrivate *priv; */ > + > + SpiceAudioPrivate *priv; > }; > > /** > @@ -67,7 +68,7 @@ struct _SpiceAudioClass { > /*< private >*/ > gboolean (*connect_channel)(SpiceAudio *audio, SpiceChannel *channel); > > - gchar _spice_reserved[SPICE_RESERVED_PADDING - sizeof(void*)]; > + gchar _spice_reserved[SPICE_RESERVED_PADDING]; > }; > > GType spice_audio_get_type(void); > diff --git a/gtk/spice-channel.h b/gtk/spice-channel.h > index f722c99..982b73b 100644 > --- a/gtk/spice-channel.h > +++ b/gtk/spice-channel.h > @@ -72,29 +72,29 @@ struct _SpiceChannelClass > { > GObjectClass parent_class; > > + /*< public >*/ > + /* signals, main context */ > + void (*channel_event)(SpiceChannel *channel, SpiceChannelEvent event); > + void (*open_fd)(SpiceChannel *channel, int with_tls); > + > /*< private >*/ > /* virtual methods, coroutine context */ > void (*handle_msg)(SpiceChannel *channel, SpiceMsgIn *msg); > void (*channel_up)(SpiceChannel *channel); > void (*iterate_write)(SpiceChannel *channel); > void (*iterate_read)(SpiceChannel *channel); > - void (*channel_reset_capabilities)(SpiceChannel *channel); > - > - /*< public >*/ > - /* signals, main context */ > - void (*channel_event)(SpiceChannel *channel, SpiceChannelEvent event); > - void (*open_fd)(SpiceChannel *channel, int with_tls); > > /*< private >*/ > /* virtual method, any context */ > void (*channel_disconnect)(SpiceChannel *channel); > - > void (*channel_reset)(SpiceChannel *channel, gboolean migrating); > + void (*channel_reset_capabilities)(SpiceChannel *channel); > + > /* > * If adding fields to this struct, remove corresponding > * amount of padding to avoid changing overall struct size > */ > - gchar _spice_reserved[SPICE_RESERVED_PADDING - 5 * sizeof(void*)]; > + gchar _spice_reserved[SPICE_RESERVED_PADDING]; > }; > > GType spice_channel_get_type(void); > diff --git a/gtk/spice-util.h b/gtk/spice-util.h > index 5bd24d3..7a617f4 100644 > --- a/gtk/spice-util.h > +++ b/gtk/spice-util.h > @@ -37,7 +37,7 @@ gulong spice_g_signal_connect_object(gpointer instance, > g_debug(G_STRLOC " " fmt, ## __VA_ARGS__); \ > } while (0) > > -#define SPICE_RESERVED_PADDING 44 > +#define SPICE_RESERVED_PADDING (10 * sizeof(void*)) > > #ifndef SPICE_NO_DEPRECATED > #define SPICE_DEPRECATED_FOR(f) \ > diff --git a/gtk/usb-device-manager.h b/gtk/usb-device-manager.h > index ba917da..df138ee 100644 > --- a/gtk/usb-device-manager.h > +++ b/gtk/usb-device-manager.h > @@ -82,7 +82,7 @@ struct _SpiceUsbDeviceManagerClass > * If adding fields to this struct, remove corresponding > * amount of padding to avoid changing overall struct size > */ > - gchar _spice_reserved[SPICE_RESERVED_PADDING - sizeof(void*)]; > + gchar _spice_reserved[SPICE_RESERVED_PADDING]; > }; > > GType spice_usb_device_get_type(void); > -- > 1.7.10.1 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
pgpzifKGd20De.pgp
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel