----- Original Message -----
> From: "Marc-André Lureau" <marcandre.lureau@xxxxxxxxxx>
> To: spice-devel@xxxxxxxxxxxxxxxxxxxxx
> Sent: Friday, June 5, 2015 6:29:44 PM
> Subject: [PATCH spice-gtk 3/3] RFC: channel: use class private handlers field
>
> Since spice-gtk requires glib 2.28, we can now fix a small FIXME.
>
> Since G_TYPE_CLASS_GET_PRIVATE is a bit expensive, it's still worth to
> cache it in klass->priv. However, there is no good place I can think of
> to put this. (channel_class_init() is called only once, and not per
> each subclass)
> ---
> src/channel-base.c | 16 +++++++++-------
> src/spice-channel-priv.h | 5 +++++
> src/spice-channel.c | 7 ++++---
> src/spice-channel.h | 4 +++-
> 4 files changed, 21 insertions(+), 11 deletions(-)
>
> diff --git a/src/channel-base.c b/src/channel-base.c
> index 77d339c..13e4ced 100644
> --- a/src/channel-base.c
> +++ b/src/channel-base.c
> @@ -197,7 +197,7 @@ end:
> }
>
>
> -static void set_handlers(SpiceChannelClass *klass,
> +static void set_handlers(SpiceChannelClassPrivate *klass,
> const spice_msg_handler* handlers, const int n)
> {
> int i;
> @@ -209,7 +209,7 @@ static void set_handlers(SpiceChannelClass *klass,
> }
> }
>
> -static void spice_channel_add_base_handlers(SpiceChannelClass *klass)
> +static void spice_channel_add_base_handlers(SpiceChannelClassPrivate *klass)
> {
> static const spice_msg_handler handlers[] = {
> [ SPICE_MSG_SET_ACK ] =
> spice_channel_handle_set_ack,
> @@ -227,12 +227,14 @@ G_GNUC_INTERNAL
> void spice_channel_set_handlers(SpiceChannelClass *klass,
> const spice_msg_handler* handlers, const int
> n)
> {
> - /* FIXME: use class private (requires glib 2.24) */
> - g_return_if_fail(klass->handlers == NULL);
> - klass->handlers = g_array_sized_new(FALSE, TRUE,
> sizeof(spice_msg_handler), n);
> + klass->priv =
> + G_TYPE_CLASS_GET_PRIVATE (klass, spice_channel_get_type (),
> SpiceChannelClassPrivate);
>
> - spice_channel_add_base_handlers(klass);
> - set_handlers(klass, handlers, n);
> + g_return_if_fail(klass->priv->handlers == NULL);
> + klass->priv->handlers = g_array_sized_new(FALSE, TRUE,
> sizeof(spice_msg_handler), n);
> +
> + spice_channel_add_base_handlers(klass->priv);
> + set_handlers(klass->priv, handlers, n);
> }
>
> static void
> diff --git a/src/spice-channel-priv.h b/src/spice-channel-priv.h
> index d70cf86..436a521 100644
> --- a/src/spice-channel-priv.h
> +++ b/src/spice-channel-priv.h
> @@ -74,6 +74,11 @@ enum spice_channel_state {
> SPICE_CHANNEL_STATE_MIGRATION_HANDSHAKE,
> };
>
> +struct _SpiceChannelClassPrivate
> +{
> + GArray *handlers;
> +};
> +
> struct _SpiceChannelPrivate {
> /* swapped on migration */
> SSL_CTX *ctx;
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index 4e7d8b7..0585a01 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -76,7 +76,8 @@ static gboolean channel_connect(SpiceChannel *channel,
> gboolean tls);
> #define SPICE_CHANNEL_GET_PRIVATE(obj) \
> (G_TYPE_INSTANCE_GET_PRIVATE ((obj), SPICE_TYPE_CHANNEL,
> SpiceChannelPrivate))
>
> -G_DEFINE_TYPE(SpiceChannel, spice_channel, G_TYPE_OBJECT);
> +G_DEFINE_TYPE_WITH_CODE (SpiceChannel, spice_channel, G_TYPE_OBJECT,
> + g_type_add_class_private (g_define_type_id, sizeof
> (SpiceChannelClassPrivate)));
>
> /* Properties */
> enum {
> @@ -2848,11 +2849,11 @@ static void spice_channel_handle_msg(SpiceChannel
> *channel, SpiceMsgIn *msg)
> int type = spice_msg_in_type(msg);
> spice_msg_handler handler;
>
> - g_return_if_fail(type < klass->handlers->len);
> + g_return_if_fail(type < klass->priv->handlers->len);
> if (type > SPICE_MSG_BASE_LAST && channel->priv->disable_channel_msg)
> return;
>
> - handler = g_array_index(klass->handlers, spice_msg_handler, type);
> + handler = g_array_index(klass->priv->handlers, spice_msg_handler, type);
> g_return_if_fail(handler != NULL);
> handler(channel, msg);
> }
> diff --git a/src/spice-channel.h b/src/spice-channel.h
> index 7f132f6..e0ce443 100644
> --- a/src/spice-channel.h
> +++ b/src/spice-channel.h
> @@ -68,6 +68,8 @@ struct _SpiceChannel
> /* Do not add fields to this struct */
> };
>
> +typedef struct _SpiceChannelClassPrivate SpiceChannelClassPrivate;
> +
> struct _SpiceChannelClass
> {
> GObjectClass parent_class;
> @@ -94,7 +96,7 @@ struct _SpiceChannelClass
> /* virtual methods, coroutine context */
> void (*channel_send_migration_handshake)(SpiceChannel *channel);
>
> - GArray *handlers;
> + SpiceChannelClassPrivate *priv;
> /*
> * If adding fields to this struct, remove corresponding
> * amount of padding to avoid changing overall struct size
> --
> 2.4.2
>
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
>
Looks good to me, but I didn't test the patch.
Consider it as an ACK if you have it tested.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel