[Spice-devel] [spice-server 2/8] Move RedChannel::data to ClientCbs::cbs_data

Frediano Ziglio fziglio at redhat.com
Tue Mar 15 21:11:36 UTC 2016


> 
> It's only used by these callbacks, moving it there makes things clearer
> about its intended use.
> ---
>  server/inputs-channel.c |  5 +++--
>  server/main-channel.c   |  2 +-
>  server/red-channel.c    | 24 +++++++++++++++++-------
>  server/red-channel.h    |  9 ++++-----
>  server/red-qxl.c        | 25 ++++++++++++-------------
>  server/reds.c           | 23 ++++++++++++++++-------
>  server/smartcard.c      |  2 +-
>  server/sound.c          | 26 +++++++++++++-------------
>  server/spicevmc.c       |  2 +-
>  9 files changed, 68 insertions(+), 50 deletions(-)
> 
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index 45e0a8f..4443825 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -531,7 +531,8 @@ static void
> inputs_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item)
>  static void inputs_connect(RedChannel *channel, RedClient *client,
>                             RedsStream *stream, int migration,
>                             int num_common_caps, uint32_t *common_caps,
> -                           int num_caps, uint32_t *caps)
> +                           int num_caps, uint32_t *caps,
> +                           gpointer cbs_data)
>  {
>      InputsChannelClient *icc;
>  
> @@ -555,7 +556,7 @@ static void inputs_connect(RedChannel *channel, RedClient
> *client,
>      inputs_pipe_add_init(&icc->base);
>  }
>  
> -static void inputs_migrate(RedChannelClient *rcc)
> +static void inputs_migrate(RedChannelClient *rcc, gpointer cbs_data)
>  {
>      InputsChannel *inputs = SPICE_CONTAINEROF(rcc->channel, InputsChannel,
>      base);
>      inputs->src_during_migrate = TRUE;
> diff --git a/server/main-channel.c b/server/main-channel.c
> index a9d0ce1..baa2033 100644
> --- a/server/main-channel.c
> +++ b/server/main-channel.c
> @@ -1148,7 +1148,7 @@ uint64_t
> main_channel_client_get_roundtrip_ms(MainChannelClient *mcc)
>      return mcc->latency / 1000;
>  }
>  
> -static void main_channel_client_migrate(RedChannelClient *rcc)
> +static void main_channel_client_migrate(RedChannelClient *rcc, gpointer
> cbs_data)
>  {
>      reds_on_main_channel_migrate(rcc->channel->reds, SPICE_CONTAINEROF(rcc,
>      MainChannelClient, base));
>      red_channel_client_default_migrate(rcc);
> diff --git a/server/red-channel.c b/server/red-channel.c
> index d8f1d27..8c4d0ba 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -992,12 +992,12 @@ static void
> red_channel_client_default_connect(RedChannel *channel, RedClient *c
>                                                 RedsStream *stream,
>                                                 int migration,
>                                                 int num_common_caps, uint32_t
>                                                 *common_caps,
> -                                               int num_caps, uint32_t *caps)
> +                                               int num_caps, uint32_t *caps,
> gpointer cbs_data)
>  {
>      spice_error("not implemented");
>  }
>  
> -static void red_channel_client_default_disconnect(RedChannelClient *base)
> +static void red_channel_client_default_disconnect(RedChannelClient *base,
> gpointer cbs_data)
>  {
>      red_channel_client_disconnect(base);
>  }
> @@ -1062,7 +1062,7 @@ RedChannel *red_channel_create(int size,
>  
>      client_cbs.connect = red_channel_client_default_connect;
>      client_cbs.disconnect = red_channel_client_default_disconnect;
> -    client_cbs.migrate = red_channel_client_default_migrate;
> +    client_cbs.migrate =
> (channel_client_migrate_proc)red_channel_client_default_migrate;
> 

I don't like these kind of cast. Why don't you add a parameter (unused)
to these functions?
 
>      red_channel_register_client_cbs(channel, &client_cbs, NULL);
>      red_channel_set_common_cap(channel, SPICE_COMMON_CAP_MINI_HEADER);
> @@ -1113,7 +1113,7 @@ RedChannel *red_channel_create_dummy(int size,
> RedsState *reds, uint32_t type, u
>      ring_init(&channel->clients);
>      client_cbs.connect = red_channel_client_default_connect;
>      client_cbs.disconnect = red_channel_client_default_disconnect;
> -    client_cbs.migrate = red_channel_client_default_migrate;
> +    client_cbs.migrate =
> (channel_client_migrate_proc)red_channel_client_default_migrate;
>  
>      red_channel_register_client_cbs(channel, &client_cbs, NULL);
>      red_channel_set_common_cap(channel, SPICE_COMMON_CAP_MINI_HEADER);
> @@ -1183,7 +1183,7 @@ void red_channel_register_client_cbs(RedChannel
> *channel, const ClientCbs *clien
>      if (client_cbs->migrate) {
>          channel->client_cbs.migrate = client_cbs->migrate;
>      }
> -    channel->data = cbs_data;
> +    channel->client_cbs.cbs_data = cbs_data;
>  }
>  
>  int test_capability(const uint32_t *caps, int num_caps, uint32_t cap)
> @@ -2106,6 +2106,16 @@ void red_client_set_migration_seamless(RedClient
> *client) // dest
>      pthread_mutex_unlock(&client->lock);
>  }
>  
> +static void red_channel_client_migrate_client(RedChannelClient *rcc)
> +{
> +    rcc->channel->client_cbs.migrate(rcc,
> rcc->channel->client_cbs.cbs_data);
> +}
> +
> +static void red_channel_client_disconnect_client(RedChannelClient *rcc)
> +{
> +    rcc->channel->client_cbs.disconnect(rcc,
> rcc->channel->client_cbs.cbs_data);
> +}
> +
>  void red_client_migrate(RedClient *client)
>  {
>      RingItem *link, *next;
> @@ -2121,7 +2131,7 @@ void red_client_migrate(RedClient *client)
>      RING_FOREACH_SAFE(link, next, &client->channels) {
>          rcc = SPICE_CONTAINEROF(link, RedChannelClient, client_link);
>          if (red_channel_client_is_connected(rcc)) {
> -            rcc->channel->client_cbs.migrate(rcc);
> +            red_channel_client_migrate_client(rcc);
>          }
>      }
>  }
> @@ -2149,7 +2159,7 @@ void red_client_destroy(RedClient *client)
>          // to wait for disconnection)
>          // TODO: should we go back to async. For this we need to use
>          // ref count for channel clients.
> -        rcc->channel->client_cbs.disconnect(rcc);
> +        red_channel_client_disconnect_client(rcc);
>          spice_assert(ring_is_empty(&rcc->pipe));
>          spice_assert(rcc->pipe_size == 0);
>          spice_assert(rcc->send_data.size == 0);
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 26304bf..51d606e 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -185,9 +185,9 @@ typedef uint64_t
> (*channel_handle_migrate_data_get_serial_proc)(RedChannelClient
>  
>  typedef void (*channel_client_connect_proc)(RedChannel *channel, RedClient
>  *client, RedsStream *stream,
>                                              int migration, int
>                                              num_common_cap, uint32_t
>                                              *common_caps,
> -                                            int num_caps, uint32_t *caps);
> -typedef void (*channel_client_disconnect_proc)(RedChannelClient *base);
> -typedef void (*channel_client_migrate_proc)(RedChannelClient *base);
> +                                            int num_caps, uint32_t *caps,
> gpointer cbs_data);
> +typedef void (*channel_client_disconnect_proc)(RedChannelClient *base,
> gpointer cbs_data);
> +typedef void (*channel_client_migrate_proc)(RedChannelClient *base, gpointer
> cbs_data);
>  
>  // TODO: add ASSERTS for thread_id  in client and channel calls
>  //
> @@ -217,6 +217,7 @@ typedef struct {
>      channel_client_connect_proc connect;
>      channel_client_disconnect_proc disconnect;
>      channel_client_migrate_proc migrate;
> +    gpointer cbs_data;
>  } ClientCbs;
>  
>  typedef struct RedChannelCapabilities {
> @@ -335,8 +336,6 @@ struct RedChannel {
>      RedChannelCapabilities local_caps;
>      uint32_t migration_flags;
>  
> -    void *data;
> -
>      // TODO: when different channel_clients are in different threads from
>      Channel -> need to protect!
>      pthread_t thread_id;
>      struct RedsState *reds;
> diff --git a/server/red-qxl.c b/server/red-qxl.c
> index ee3cab0..f720fd5 100644
> --- a/server/red-qxl.c
> +++ b/server/red-qxl.c
> @@ -75,13 +75,13 @@ static int red_qxl_check_qxl_version(QXLState *rq, int
> major, int minor)
>  static void red_qxl_set_display_peer(RedChannel *channel, RedClient *client,
>                                       RedsStream *stream, int migration,
>                                       int num_common_caps, uint32_t
>                                       *common_caps, int num_caps,
> -                                     uint32_t *caps)
> +                                     uint32_t *caps, gpointer cbs_data)
>  {
>      RedWorkerMessageDisplayConnect payload = {0,};
>      QXLState *qxl_state;
>  
>      spice_debug("%s", "");
> -    qxl_state = (QXLState *)channel->data;
> +    qxl_state = (QXLState *)cbs_data;
>      payload.client = client;
>      payload.stream = stream;
>      payload.migration = migration;
> @@ -98,7 +98,7 @@ static void red_qxl_set_display_peer(RedChannel *channel,
> RedClient *client,
>                              &payload);
>  }
>  
> -static void red_qxl_disconnect_display_peer(RedChannelClient *rcc)
> +static void red_qxl_disconnect_display_peer(RedChannelClient *rcc, gpointer
> cbs_data)
>  {
>      RedWorkerMessageDisplayDisconnect payload;
>      QXLState *qxl_state;
> @@ -107,8 +107,7 @@ static void
> red_qxl_disconnect_display_peer(RedChannelClient *rcc)
>          return;
>      }
>  
> -    qxl_state = (QXLState *)rcc->channel->data;
> -
> +    qxl_state = (QXLState *)cbs_data;
>      spice_printerr("");
>      payload.rcc = rcc;
>  
> @@ -119,14 +118,14 @@ static void
> red_qxl_disconnect_display_peer(RedChannelClient *rcc)
>                              &payload);
>  }
>  
> -static void red_qxl_display_migrate(RedChannelClient *rcc)
> +static void red_qxl_display_migrate(RedChannelClient *rcc, gpointer
> cbs_data)
>  {
>      RedWorkerMessageDisplayMigrate payload;
>      QXLState *qxl_state;
>      if (!rcc->channel) {
>          return;
>      }
> -    qxl_state = (QXLState *)rcc->channel->data;
> +    qxl_state = (QXLState *)cbs_data;
>      spice_printerr("channel type %u id %u", rcc->channel->type,
>      rcc->channel->id);
>      payload.rcc = rcc;
>      dispatcher_send_message(&qxl_state->dispatcher,
> @@ -137,10 +136,10 @@ static void red_qxl_display_migrate(RedChannelClient
> *rcc)
>  static void red_qxl_set_cursor_peer(RedChannel *channel, RedClient *client,
>  RedsStream *stream,
>                                      int migration, int num_common_caps,
>                                      uint32_t *common_caps, int num_caps,
> -                                    uint32_t *caps)
> +                                    uint32_t *caps, gpointer cbs_data)
>  {
>      RedWorkerMessageCursorConnect payload = {0,};
> -    QXLState *qxl_state = (QXLState *)channel->data;
> +    QXLState *qxl_state = (QXLState *)cbs_data;
>      spice_printerr("");
>      payload.client = client;
>      payload.stream = stream;
> @@ -158,7 +157,7 @@ static void red_qxl_set_cursor_peer(RedChannel *channel,
> RedClient *client, Reds
>                              &payload);
>  }
>  
> -static void red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
> +static void red_qxl_disconnect_cursor_peer(RedChannelClient *rcc, gpointer
> cbs_data)
>  {
>      RedWorkerMessageCursorDisconnect payload;
>      QXLState *qxl_state;
> @@ -167,7 +166,7 @@ static void
> red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
>          return;
>      }
>  
> -    qxl_state = (QXLState *)rcc->channel->data;
> +    qxl_state = (QXLState *)cbs_data;
>      spice_printerr("");
>      payload.rcc = rcc;
>  
> @@ -176,7 +175,7 @@ static void
> red_qxl_disconnect_cursor_peer(RedChannelClient *rcc)
>                              &payload);
>  }
>  
> -static void red_qxl_cursor_migrate(RedChannelClient *rcc)
> +static void red_qxl_cursor_migrate(RedChannelClient *rcc, gpointer cbs_data)
>  {
>      RedWorkerMessageCursorMigrate payload;
>      QXLState *qxl_state;
> @@ -184,7 +183,7 @@ static void red_qxl_cursor_migrate(RedChannelClient *rcc)
>      if (!rcc->channel) {
>          return;
>      }
> -    qxl_state = (QXLState *)rcc->channel->data;
> +    qxl_state = (QXLState *)cbs_data;
>      spice_printerr("channel type %u id %u", rcc->channel->type,
>      rcc->channel->id);
>      payload.rcc = rcc;
>      dispatcher_send_message(&qxl_state->dispatcher,
> diff --git a/server/reds.c b/server/reds.c
> index b1e1139..5d0a66e 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -1730,6 +1730,15 @@ static void openssl_init(RedLinkInfo *link)
>      BN_set_word(link->tiTicketing.bn, f4);
>  }
>  
> +static void red_channel_connect_client(RedChannel *channel, RedClient
> *client, RedsStream *stream,
> +                                       int migration, int num_common_caps,
> uint32_t *common_caps,
> +                                       int num_caps, uint32_t *caps)
> +{
> +    channel->client_cbs.connect(channel, client, stream, migration,
> +                                num_common_caps, common_caps, num_caps,
> caps,
> +                                channel->client_cbs.cbs_data);
> +}
> +
>  static void reds_channel_do_link(RedChannel *channel, RedClient *client,
>                                   SpiceLinkMess *link_msg,
>                                   RedsStream *stream)
> @@ -1741,13 +1750,13 @@ static void reds_channel_do_link(RedChannel *channel,
> RedClient *client,
>      spice_assert(stream);
>  
>      caps = (uint32_t *)((uint8_t *)link_msg + link_msg->caps_offset);
> -    channel->client_cbs.connect(channel, client, stream,
> -                                red_client_during_migrate_at_target(client),
> -                                link_msg->num_common_caps,
> -                                link_msg->num_common_caps ? caps : NULL,
> -                                link_msg->num_channel_caps,
> -                                link_msg->num_channel_caps ?
> -                                caps + link_msg->num_common_caps : NULL);
> +    red_channel_connect_client(channel, client, stream,
> +                               red_client_during_migrate_at_target(client),
> +                               link_msg->num_common_caps,
> +                               link_msg->num_common_caps ? caps : NULL,
> +                               link_msg->num_channel_caps,
> +                               link_msg->num_channel_caps ?
> +                               caps + link_msg->num_common_caps : NULL);
>  }
>  
>  /*
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 2b25bac..4d06784 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -802,7 +802,7 @@ static void
> smartcard_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *it
>  static void smartcard_connect_client(RedChannel *channel, RedClient *client,
>                                       RedsStream *stream, int migration,
>                                       int num_common_caps, uint32_t
>                                       *common_caps,
> -                                     int num_caps, uint32_t *caps)
> +                                     int num_caps, uint32_t *caps, gpointer
> cbs_data)
>  {
>      SpiceCharDeviceInstance *char_device =
>              smartcard_readers_get_unattached();
> diff --git a/server/sound.c b/server/sound.c
> index 82c14f7..672d23f 100644
> --- a/server/sound.c
> +++ b/server/sound.c
> @@ -987,13 +987,13 @@ error1:
>      return NULL;
>  }
>  
> -static void snd_disconnect_channel_client(RedChannelClient *rcc)
> +static void snd_disconnect_channel_client(RedChannelClient *rcc, gpointer
> cbs_data)
>  {
>      SndWorker *worker;
>  
>      spice_assert(rcc->channel);
> -    spice_assert(rcc->channel->data);
> -    worker = (SndWorker *)rcc->channel->data;
> +    spice_assert(cbs_data);
> +    worker = (SndWorker *)cbs_data;
>  
>      spice_debug("channel-type=%d", rcc->channel->type);
>      if (worker->connection) {
> @@ -1202,9 +1202,9 @@ static void snd_playback_cleanup(SndChannel *channel)
>  
>  static void snd_set_playback_peer(RedChannel *channel, RedClient *client,
>  RedsStream *stream,
>                                    int migration, int num_common_caps,
>                                    uint32_t *common_caps,
> -                                  int num_caps, uint32_t *caps)
> +                                  int num_caps, uint32_t *caps, gpointer
> cbs_data)
>  {
> -    SndWorker *worker = channel->data;
> +    SndWorker *worker = cbs_data;
>      PlaybackChannel *playback_channel;
>      SpicePlaybackState *st = SPICE_CONTAINEROF(worker, SpicePlaybackState,
>      worker);
>  
> @@ -1253,14 +1253,14 @@ static void snd_set_playback_peer(RedChannel
> *channel, RedClient *client, RedsSt
>      snd_playback_send(worker->connection);
>  }
>  
> -static void snd_record_migrate_channel_client(RedChannelClient *rcc)
> +static void snd_record_migrate_channel_client(RedChannelClient *rcc,
> gpointer cbs_data)
>  {
>      SndWorker *worker;
>  
>      spice_debug(NULL);
>      spice_assert(rcc->channel);
> -    spice_assert(rcc->channel->data);
> -    worker = (SndWorker *)rcc->channel->data;
> +    spice_assert(cbs_data);
> +    worker = (SndWorker *)cbs_data;
>  
>      if (worker->connection) {
>          spice_assert(worker->connection->channel_client == rcc);
> @@ -1448,9 +1448,9 @@ static void snd_record_cleanup(SndChannel *channel)
>  
>  static void snd_set_record_peer(RedChannel *channel, RedClient *client,
>  RedsStream *stream,
>                                  int migration, int num_common_caps, uint32_t
>                                  *common_caps,
> -                                int num_caps, uint32_t *caps)
> +                                int num_caps, uint32_t *caps, gpointer
> cbs_data)
>  {
> -    SndWorker *worker = channel->data;
> +    SndWorker *worker = cbs_data;
>      RecordChannel *record_channel;
>      SpiceRecordState *st = SPICE_CONTAINEROF(worker, SpiceRecordState,
>      worker);
>  
> @@ -1482,13 +1482,13 @@ static void snd_set_record_peer(RedChannel *channel,
> RedClient *client, RedsStre
>      snd_record_send(worker->connection);
>  }
>  
> -static void snd_playback_migrate_channel_client(RedChannelClient *rcc)
> +static void snd_playback_migrate_channel_client(RedChannelClient *rcc,
> gpointer cbs_data)
>  {
>      SndWorker *worker;
>  
>      spice_assert(rcc->channel);
> -    spice_assert(rcc->channel->data);
> -    worker = (SndWorker *)rcc->channel->data;
> +    spice_assert(cbs_data);
> +    worker = (SndWorker *)cbs_data;
>      spice_debug(NULL);
>  
>      if (worker->connection) {
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index f745fdb..43c1eff 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -453,7 +453,7 @@ static void
> spicevmc_red_channel_release_pipe_item(RedChannelClient *rcc,
>  
>  static void spicevmc_connect(RedChannel *channel, RedClient *client,
>      RedsStream *stream, int migration, int num_common_caps,
> -    uint32_t *common_caps, int num_caps, uint32_t *caps)
> +    uint32_t *common_caps, int num_caps, uint32_t *caps, gpointer cbs_data)
>  {
>      RedChannelClient *rcc;
>      SpiceVmcState *state;

Frediano


More information about the Spice-devel mailing list