[Spice-devel] [PATCH spice-gtk] Running out of reserved space, break ABI

Marc-André Lureau marcandre.lureau at gmail.com
Thu May 17 07:20:12 PDT 2012


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:
- 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
- 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 */
 };
 
 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



More information about the Spice-devel mailing list