> > 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 ;) > Yes, I know, was mainly a pending follow up. > 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. > Agreed > 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... > Disagreed, is not a class thing, it's should be a responsibility of however assign the thread to the specific instance. Is not much thread local, more "instance local" I would say. > 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; _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel