I have an alternate proposal that I'll send in a little bit.
On Fri, 2017-08-25 at 15:29 +0100, Frediano Ziglio wrote:
> Instead of using a separate parameter and field store
> in the same structure.
> This allows to pass the pointer while passing callbacks
> and also make easier to understand that "data" field
> is related to callbacks.
> Also allows to retrieve client callback saved pointer.
> The data pointer for client callbacks was not used.
> The code was saving this information using callback registration
> and g_object_set_data. Remove the g_object_set_data using
> the data registered.
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/inputs-channel.c | 2 +-
> server/main-channel.c | 2 +-
> server/red-channel.c | 12 +++++++-----
> server/red-channel.h | 5 ++++-
> server/red-qxl.c | 14 ++++++++------
> server/red-worker.c | 6 ++----
> server/smartcard.c | 2 +-
> server/sound.c | 4 ++--
> server/spicevmc.c | 2 +-
> 9 files changed, 27 insertions(+), 22 deletions(-)
>
> Changes since v1:
> - remove parameter and store in ClientCbs;
> - rename function to get this pointer.
>
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index 943c69d6..b7abe216 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -553,7 +553,7 @@ inputs_channel_constructed(GObject *object)
>
> client_cbs.connect = inputs_connect;
> client_cbs.migrate = inputs_migrate;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> NULL);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
>
> red_channel_set_cap(RED_CHANNEL(self),
> SPICE_INPUTS_CAP_KEY_SCANCODE);
> reds_register_channel(reds, RED_CHANNEL(self));
> diff --git a/server/main-channel.c b/server/main-channel.c
> index 4834f79b..6eb41556 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -295,7 +295,7 @@ main_channel_constructed(GObject *object)
> red_channel_set_cap(RED_CHANNEL(self),
> SPICE_MAIN_CAP_SEAMLESS_MIGRATE);
>
> client_cbs.migrate = main_channel_client_migrate;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> NULL);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
> }
>
> static void
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 9ff3474a..e19fed93 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -91,8 +91,6 @@ struct RedChannelPrivate
> RedChannelCapabilities local_caps;
> uint32_t migration_flags;
>
> - void *data;
> -
> ClientCbs client_cbs;
> // TODO: when different channel_clients are in different threads
> // from Channel -> need to protect!
> @@ -356,8 +354,7 @@ const RedStatNode
> *red_channel_get_stat_node(RedChannel *channel)
> return &channel->priv->stat;
> }
>
> -void red_channel_register_client_cbs(RedChannel *channel, const
> ClientCbs *client_cbs,
> - gpointer cbs_data)
> +void red_channel_register_client_cbs(RedChannel *channel, const
> ClientCbs *client_cbs)
> {
> spice_assert(client_cbs->connect || channel->priv->type ==
> SPICE_CHANNEL_MAIN);
> channel->priv->client_cbs.connect = client_cbs->connect;
> @@ -369,7 +366,12 @@ void red_channel_register_client_cbs(RedChannel
> *channel, const ClientCbs *clien
> if (client_cbs->migrate) {
> channel->priv->client_cbs.migrate = client_cbs->migrate;
> }
> - channel->priv->data = cbs_data;
> + channel->priv->client_cbs.data = client_cbs->data;
> +}
> +
> +void *red_channel_get_client_cbs_data(RedChannel *channel)
> +{
> + return channel->priv->client_cbs.data;
> }
>
> static void add_capability(uint32_t **caps, int *num_caps, uint32_t
> cap)
> diff --git a/server/red-channel.h b/server/red-channel.h
> index e65eea1e..559dd2b3 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -70,6 +70,7 @@ typedef struct {
> channel_client_connect_proc connect;
> channel_client_disconnect_proc disconnect;
> channel_client_migrate_proc migrate;
> + void *data;
> } ClientCbs;
>
> static inline gboolean test_capability(const uint32_t *caps, int
> num_caps, uint32_t cap)
> @@ -135,7 +136,9 @@ void red_channel_remove_client(RedChannel
> *channel, RedChannelClient *rcc);
>
> void red_channel_init_stat_node(RedChannel *channel, const
> RedStatNode *parent, const char *name);
>
> -void red_channel_register_client_cbs(RedChannel *channel, const
> ClientCbs *client_cbs, gpointer cbs_data);
> +void red_channel_register_client_cbs(RedChannel *channel, const
> ClientCbs *client_cbs);
> +// retrieve data pointer stored with red_channel_register_client_cbs
> +void *red_channel_get_client_cbs_data(RedChannel *channel);
> // caps are freed when the channel is destroyed
> void red_channel_set_common_cap(RedChannel *channel, uint32_t cap);
> void red_channel_set_cap(RedChannel *channel, uint32_t cap);
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index ba869e54..cdb04b23 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -82,7 +82,7 @@ static void red_qxl_set_display_peer(RedChannel
> *channel, RedClient *client,
> Dispatcher *dispatcher;
>
> spice_debug("%s", "");
> - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> + dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
> payload.client = client;
> payload.stream = stream;
> payload.migration = migration;
> @@ -99,7 +99,7 @@ static void
> red_qxl_disconnect_display_peer(RedChannelClient *rcc)
> Dispatcher *dispatcher;
> RedChannel *channel = red_channel_client_get_channel(rcc);
>
> - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> + dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
>
> spice_printerr("");
> payload.rcc = rcc;
> @@ -119,7 +119,7 @@ static void
> red_qxl_display_migrate(RedChannelClient *rcc)
> uint32_t type, id;
>
> g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> + dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
> spice_printerr("channel type %u id %u", type, id);
> payload.rcc = rcc;
> dispatcher_send_message(dispatcher,
> @@ -132,7 +132,7 @@ static void red_qxl_set_cursor_peer(RedChannel
> *channel, RedClient *client, Reds
> RedChannelCapabilities *caps)
> {
> RedWorkerMessageCursorConnect payload = {0,};
> - Dispatcher *dispatcher = (Dispatcher
> *)g_object_get_data(G_OBJECT(channel), "dispatcher");
> + Dispatcher *dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
> spice_printerr("");
> payload.client = client;
> payload.stream = stream;
> @@ -150,7 +150,7 @@ static void
> red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
> Dispatcher *dispatcher;
> RedChannel *channel = red_channel_client_get_channel(rcc);
>
> - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> + dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
> spice_printerr("");
> payload.rcc = rcc;
>
> @@ -167,7 +167,7 @@ static void
> red_qxl_cursor_migrate(RedChannelClient *rcc)
> uint32_t type, id;
>
> g_object_get(channel, "channel-type", &type, "id", &id, NULL);
> - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel),
> "dispatcher");
> + dispatcher = (Dispatcher
> *)red_channel_get_client_cbs_data(channel);
> spice_printerr("channel type %u id %u", type, id);
> payload.rcc = rcc;
> dispatcher_send_message(dispatcher,
> @@ -976,10 +976,12 @@ void red_qxl_init(RedsState *reds, QXLInstance
> *qxl)
> client_cursor_cbs.connect = red_qxl_set_cursor_peer;
> client_cursor_cbs.disconnect = red_qxl_disconnect_cursor_peer;
> client_cursor_cbs.migrate = red_qxl_cursor_migrate;
> + client_cursor_cbs.data = qxl_state->dispatcher;
>
> client_display_cbs.connect = red_qxl_set_display_peer;
> client_display_cbs.disconnect = red_qxl_disconnect_display_peer;
> client_display_cbs.migrate = red_qxl_display_migrate;
> + client_display_cbs.data = qxl_state->dispatcher;
>
> qxl_state->worker = red_worker_new(qxl, &client_cursor_cbs,
> &client_display_cbs);
> diff --git a/server/red-worker.c b/server/red-worker.c
> index 03a409cd..8fd115d8 100644
> --- a/server/red-worker.c
> +++ b/server/red-worker.c
> @@ -1346,8 +1346,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> &worker->core);
> channel = RED_CHANNEL(worker->cursor_channel);
> red_channel_init_stat_node(channel, &worker->stat,
> "cursor_channel");
> - red_channel_register_client_cbs(channel, client_cursor_cbs,
> dispatcher);
> - g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
> + red_channel_register_client_cbs(channel, client_cursor_cbs);
> reds_register_channel(reds, channel);
>
> // TODO: handle seemless migration. Temp, setting migrate to
> FALSE
> @@ -1357,8 +1356,7 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> init_info.n_surfac
> es);
> channel = RED_CHANNEL(worker->display_channel);
> red_channel_init_stat_node(channel, &worker->stat,
> "display_channel");
> - red_channel_register_client_cbs(channel, client_display_cbs,
> dispatcher);
> - g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher);
> + red_channel_register_client_cbs(channel, client_display_cbs);
> reds_register_channel(reds, channel);
>
> return worker;
> diff --git a/server/smartcard.c b/server/smartcard.c
> index ac2fc1e1..3f98eb1b 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -559,7 +559,7 @@ red_smartcard_channel_constructed(GObject
> *object)
> G_OBJECT_CLASS(red_smartcard_channel_parent_class)-
> >constructed(object);
>
> client_cbs.connect = smartcard_connect_client;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> NULL);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
>
> reds_register_channel(reds, RED_CHANNEL(self));
> }
> diff --git a/server/sound.c b/server/sound.c
> index 54b89713..60016041 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -1339,7 +1339,7 @@ playback_channel_constructed(GObject *object)
>
> client_cbs.connect = snd_set_playback_peer;
> client_cbs.migrate = snd_migrate_channel_client;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
>
> if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1,
> SND_CODEC_ANY_FREQUENCY)) {
> red_channel_set_cap(RED_CHANNEL(self),
> SPICE_PLAYBACK_CAP_CELT_0_5_1);
> @@ -1389,7 +1389,7 @@ record_channel_constructed(GObject *object)
>
> client_cbs.connect = snd_set_record_peer;
> client_cbs.migrate = snd_migrate_channel_client;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> self);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
>
> if (snd_codec_is_capable(SPICE_AUDIO_DATA_MODE_CELT_0_5_1,
> SND_CODEC_ANY_FREQUENCY)) {
> red_channel_set_cap(RED_CHANNEL(self),
> SPICE_RECORD_CAP_CELT_0_5_1);
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 9305c9b4..0f7145c6 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -231,7 +231,7 @@ red_vmc_channel_constructed(GObject *object)
> G_OBJECT_CLASS(red_vmc_channel_parent_class)-
> >constructed(object);
>
> client_cbs.connect = spicevmc_connect;
> - red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs,
> NULL);
> + red_channel_register_client_cbs(RED_CHANNEL(self), &client_cbs);
>
> red_channel_init_stat_node(RED_CHANNEL(self), NULL, "spicevmc");
> const RedStatNode *stat =
> red_channel_get_stat_node(RED_CHANNEL(self));
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel