Hey, This loops back on that old/unresolved discussion https://lists.freedesktop.org/archives/spice-devel/2017-August/039541.html I'd rather not make changes like this for now as we don't know where we want to go with these ClientCbs ;) The only reason why we have these ClientCbs is to be able to marshall calls from the channel thread to the main thread if needed, and to decide whether to marshall or not, we only need to know if the current channel needs a dispatcher or not, and call the corresponding channel specific method either directly, or ensure the call gets to the correct thread. ClientCbs is definitely not something which needs to be registered per-channel instance, it's more of a class thing, and the dispatcher is more something which could be thread local. I would not stick everything together like this before cleaning up more of that mess... Christophe On Fri, Jun 22, 2018 at 09:24:09AM +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. > Remove the g_object_set_data using the data registered. > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > server/red-channel.c | 6 ++++++ > server/red-channel.h | 3 +++ > server/red-qxl.c | 14 ++++++++------ > server/red-worker.c | 2 -- > 4 files changed, 17 insertions(+), 8 deletions(-) > > diff --git a/server/red-channel.c b/server/red-channel.c > index 448b690e..1167fd79 100644 > --- a/server/red-channel.c > +++ b/server/red-channel.c > @@ -374,6 +374,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->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 4e7a9066..8d68eeb0 100644 > --- a/server/red-channel.h > +++ b/server/red-channel.h > @@ -71,6 +71,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) > @@ -123,6 +124,8 @@ 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); > +// 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 730b9f3d..4b36acdb 100644 > --- a/server/red-qxl.c > +++ b/server/red-qxl.c > @@ -78,7 +78,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); > // get a reference potentially the main channel can be destroyed in > // the main thread causing RedClient to be destroyed before using it > payload.client = g_object_ref(client); > @@ -97,7 +97,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; > @@ -117,7 +117,7 @@ static void red_qxl_display_migrate(RedChannelClient *rcc) > > red_channel_printerr(channel, ""); > > - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel), "dispatcher"); > + dispatcher = (Dispatcher *)red_channel_get_client_cbs_data(channel); > payload.rcc = g_object_ref(rcc); > dispatcher_send_message(dispatcher, > RED_WORKER_MESSAGE_DISPLAY_MIGRATE, > @@ -129,7 +129,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(""); > // get a reference potentially the main channel can be destroyed in > // the main thread causing RedClient to be destroyed before using it > @@ -149,7 +149,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; > > @@ -166,7 +166,7 @@ static void red_qxl_cursor_migrate(RedChannelClient *rcc) > > red_channel_printerr(channel, ""); > > - dispatcher = (Dispatcher *)g_object_get_data(G_OBJECT(channel), "dispatcher"); > + dispatcher = (Dispatcher *)red_channel_get_client_cbs_data(channel); > payload.rcc = g_object_ref(rcc); > dispatcher_send_message(dispatcher, > RED_WORKER_MESSAGE_CURSOR_MIGRATE, > @@ -917,10 +917,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 21ae6d77..fa3697a3 100644 > --- a/server/red-worker.c > +++ b/server/red-worker.c > @@ -1338,7 +1338,6 @@ RedWorker* red_worker_new(QXLInstance *qxl, > 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); > - g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher); > reds_register_channel(reds, channel); > > // TODO: handle seemless migration. Temp, setting migrate to FALSE > @@ -1349,7 +1348,6 @@ RedWorker* red_worker_new(QXLInstance *qxl, > 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); > - g_object_set_data(G_OBJECT(channel), "dispatcher", dispatcher); > reds_register_channel(reds, channel); > > return worker; > -- > 2.17.1 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/spice-devel
Attachment:
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel