[Spice-devel] [PATCH 13/18] RedsState: use local variables in more places

Frediano Ziglio fziglio at redhat.com
Tue Feb 9 11:54:48 UTC 2016


> 
> From: Jonathon Jongsma <jjongsma at redhat.com>
> 
> Various functions were still using the global 'reds' variable rather
> than the local argument of type RedsState

I think we decided to rename all variable/argument to reds at the end
so this patch is not doing the right thing.

> ---
>  server/reds.c | 68
>  +++++++++++++++++++++++++++++------------------------------
>  1 file changed, 34 insertions(+), 34 deletions(-)
> 
> diff --git a/server/reds.c b/server/reds.c
> index fe35d84..012c0f3 100644
> --- a/server/reds.c
> +++ b/server/reds.c
> @@ -859,7 +859,7 @@ static int reds_get_n_clients(RedsState *reds)
>  SPICE_GNUC_VISIBLE int spice_server_get_num_clients(SpiceServer *s)
>  {
>      spice_assert(reds == s);
> -    return reds_get_n_clients(reds);
> +    return reds_get_n_clients(s);
>  }
>  
>  static int secondary_channels[] = {
> @@ -1340,6 +1340,7 @@ int reds_handle_migrate_data(RedsState *reds,
> MainChannelClient *mcc,
>  
>  static void reds_channel_init_auth_caps(RedLinkInfo *link, RedChannel
>  *channel)
>  {
> +    RedsState *reds = link->reds;
>      if (reds->sasl_enabled && !link->skip_auth) {
>          red_channel_set_common_cap(channel, SPICE_COMMON_CAP_AUTH_SASL);
>      } else {

This hunk should be squashed in RedLinkInfo patch

> @@ -3116,11 +3117,11 @@ static int
> spice_server_char_device_add_interface(SpiceServer *s,
>          spice_assert(char_device->st);
>          /* setting the char_device state to "started" for backward
>          compatibily with
>           * qemu releases that don't call spice api for start/stop (not
>           implemented yet) */
> -        if (reds->vm_running) {
> +        if (s->vm_running) {
>              spice_char_device_start(char_device->st);
>          }
>          spice_char_device_set_server(char_device->st, reds);
> -        reds_char_device_add_state(reds, char_device->st);
> +        reds_char_device_add_state(s, char_device->st);
>      } else {
>          spice_warning("failed to create device state for %s",
>          char_device->subtype);
>          return -1;
> @@ -3740,7 +3741,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_set_channel_security(SpiceServer *s, const c
>  SPICE_GNUC_VISIBLE int spice_server_get_sock_info(SpiceServer *s, struct
>  sockaddr *sa, socklen_t *salen)
>  {
>      spice_assert(reds == s);
> -    if (main_channel_getsockname(reds->main_channel, sa, salen) < 0) {
> +    if (main_channel_getsockname(s->main_channel, sa, salen) < 0) {
>          return -1;
>      }
>      return 0;
> @@ -3749,7 +3750,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_get_sock_info(SpiceServer *s, struct sockadd
>  SPICE_GNUC_VISIBLE int spice_server_get_peer_info(SpiceServer *s, struct
>  sockaddr *sa, socklen_t *salen)
>  {
>      spice_assert(reds == s);
> -    if (main_channel_getpeername(reds->main_channel, sa, salen) < 0) {
> +    if (main_channel_getpeername(s->main_channel, sa, salen) < 0) {
>          return -1;
>      }
>      return 0;
> @@ -3758,7 +3759,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_get_peer_info(SpiceServer *s, struct sockadd
>  SPICE_GNUC_VISIBLE int spice_server_is_server_mouse(SpiceServer *s)
>  {
>      spice_assert(reds == s);
> -    return reds->mouse_mode == SPICE_MOUSE_MODE_SERVER;
> +    return s->mouse_mode == SPICE_MOUSE_MODE_SERVER;
>  }
>  
>  SPICE_GNUC_VISIBLE int spice_server_add_renderer(SpiceServer *s, const char
>  *name)
> @@ -3802,7 +3803,6 @@ SPICE_GNUC_VISIBLE int
> spice_server_set_playback_compression(SpiceServer *s, int
>  
>  SPICE_GNUC_VISIBLE int spice_server_set_agent_mouse(SpiceServer *s, int
>  enable)
>  {
> -    spice_assert(reds == s);
>      reds->agent_mouse = enable;
>      reds_update_mouse_mode(reds);
>      return 0;
> @@ -3812,8 +3812,8 @@ SPICE_GNUC_VISIBLE int
> spice_server_set_agent_copypaste(SpiceServer *s, int enab
>  {
>      spice_assert(reds == s);
>      s->agent_copypaste = enable;
> -    reds->agent_state.write_filter.copy_paste_enabled = s->agent_copypaste;
> -    reds->agent_state.read_filter.copy_paste_enabled = s->agent_copypaste;
> +    s->agent_state.write_filter.copy_paste_enabled = s->agent_copypaste;
> +    s->agent_state.read_filter.copy_paste_enabled = s->agent_copypaste;
>      return 0;
>  }
>  
> @@ -3821,8 +3821,8 @@ SPICE_GNUC_VISIBLE int
> spice_server_set_agent_file_xfer(SpiceServer *s, int enab
>  {
>      spice_assert(reds == s);
>      s->agent_file_xfer = enable;
> -    reds->agent_state.write_filter.file_xfer_enabled = s->agent_file_xfer;
> -    reds->agent_state.read_filter.file_xfer_enabled = s->agent_file_xfer;
> +    s->agent_state.write_filter.file_xfer_enabled = s->agent_file_xfer;
> +    s->agent_state.read_filter.file_xfer_enabled = s->agent_file_xfer;
>      return 0;
>  }
>  
> @@ -3863,9 +3863,9 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_connect(SpiceServer *s, const char*
>      spice_assert(s->migration_interface);
>      spice_assert(reds == s);
>  
> -    if (reds->expect_migrate) {
> +    if (s->expect_migrate) {
>          spice_info("consecutive calls without migration. Canceling previous
>          call");
> -        main_channel_migrate_src_complete(reds->main_channel, FALSE);
> +        main_channel_migrate_src_complete(s->main_channel, FALSE);
>      }
>  
>      sif = SPICE_CONTAINEROF(s->migration_interface->base.sif,
>      SpiceMigrateInterface, base);
> @@ -3875,7 +3875,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_connect(SpiceServer *s, const char*
>          return -1;
>      }
>  
> -    reds->expect_migrate = TRUE;
> +    s->expect_migrate = TRUE;
>  
>      /*
>       * seamless migration support was added to the client after the support
>       in
> @@ -3885,16 +3885,16 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_connect(SpiceServer *s, const char*
>       * occured when the agent was not connected, the tokens state after
>       migration will still
>       * be valid (see reds_reset_vdp for more details).
>       */
> -    try_seamless = reds->seamless_migration_enabled &&
> -                   red_channel_test_remote_cap(&reds->main_channel->base,
> +    try_seamless = s->seamless_migration_enabled &&
> +                   red_channel_test_remote_cap(&s->main_channel->base,
>                     SPICE_MAIN_CAP_AGENT_CONNECTED_TOKENS);
>      /* main channel will take care of clients that are still during
>      migration (at target)*/
> -    if (main_channel_migrate_connect(reds->main_channel, reds->mig_spice,
> +    if (main_channel_migrate_connect(s->main_channel, s->mig_spice,
>                                       try_seamless)) {
> -        reds_mig_started(reds);
> +        reds_mig_started(s);
>      } else {
> -        if (reds->num_clients == 0) {
> -            reds_mig_release(reds);
> +        if (s->num_clients == 0) {
> +            reds_mig_release(s);
>              spice_info("no client connected");
>          }
>          sif->migrate_connect_complete(s->migration_interface);
> @@ -3921,7 +3921,7 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_start(SpiceServer *s)
>  {
>      spice_assert(reds == s);
>      spice_info(NULL);
> -    if (!reds->mig_spice) {
> +    if (!s->mig_spice) {
>          return -1;
>      }
>      return 0;
> @@ -3938,19 +3938,19 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_end(SpiceServer *s, int completed)
>      spice_assert(reds == s);
>  
>      sif = SPICE_CONTAINEROF(s->migration_interface->base.sif,
>      SpiceMigrateInterface, base);
> -    if (completed && !reds->expect_migrate && reds->num_clients) {
> +    if (completed && !s->expect_migrate && s->num_clients) {
>          spice_warning("spice_server_migrate_info was not called,
>          disconnecting clients");
> -        reds_disconnect(reds);
> +        reds_disconnect(s);
>          ret = -1;
>          goto complete;
>      }
>  
> -    reds->expect_migrate = FALSE;
> -    if (!reds_main_channel_connected(reds)) {
> +    s->expect_migrate = FALSE;
> +    if (!reds_main_channel_connected(s)) {
>          spice_info("no peer connected");
>          goto complete;
>      }
> -    reds_mig_finished(reds, completed);
> +    reds_mig_finished(s, completed);
>      return 0;
>  complete:
>      if (sif->migrate_end_complete) {
> @@ -3964,11 +3964,11 @@ SPICE_GNUC_VISIBLE int
> spice_server_migrate_switch(SpiceServer *s)
>  {
>      spice_assert(reds == s);
>      spice_info(NULL);
> -    if (!reds->num_clients) {
> +    if (!s->num_clients) {
>         return 0;
>      }
> -    reds->expect_migrate = FALSE;
> -    reds_mig_switch(reds);
> +    s->expect_migrate = FALSE;
> +    reds_mig_switch(s);
>      return 0;
>  }
>  
> @@ -3977,8 +3977,8 @@ SPICE_GNUC_VISIBLE void
> spice_server_vm_start(SpiceServer *s)
>      RingItem *item;
>  
>      spice_assert(s == reds);
> -    reds->vm_running = TRUE;
> -    RING_FOREACH(item, &reds->char_devs_states) {
> +    s->vm_running = TRUE;
> +    RING_FOREACH(item, &s->char_devs_states) {
>          SpiceCharDeviceStateItem *st_item;
>  
>          st_item = SPICE_CONTAINEROF(item, SpiceCharDeviceStateItem, link);
> @@ -3992,8 +3992,8 @@ SPICE_GNUC_VISIBLE void
> spice_server_vm_stop(SpiceServer *s)
>      RingItem *item;
>  
>      spice_assert(s == reds);
> -    reds->vm_running = FALSE;
> -    RING_FOREACH(item, &reds->char_devs_states) {
> +    s->vm_running = FALSE;
> +    RING_FOREACH(item, &s->char_devs_states) {
>          SpiceCharDeviceStateItem *st_item;
>  
>          st_item = SPICE_CONTAINEROF(item, SpiceCharDeviceStateItem, link);
> @@ -4006,7 +4006,7 @@ SPICE_GNUC_VISIBLE void
> spice_server_set_seamless_migration(SpiceServer *s, int
>  {
>      spice_assert(s == reds);
>      /* seamless migration is not supported with multiple clients */
> -    reds->seamless_migration_enabled = enable &&
> !reds->allow_multiple_clients;
> +    s->seamless_migration_enabled = enable && !s->allow_multiple_clients;
>      spice_debug("seamless migration enabled=%d", enable);
>  }
>  

I'll have to do some extra work but I think to NACK it.

Frediano


More information about the Spice-devel mailing list