Re: [PATCH v2 1/9] Avoid passing pipe item to red_channel_client_init_send_data()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



> The only time that the pipe item needs to be passed as the third
> argument to red_channel_client_init_send_data() is when the pipe item
> holds a data buffer that has been added to the marshaller by reference
> (spice_marshaller_add_by_ref()) and needs to be kept alive until the
> data has been sent. In all other cases, the item does not need to be
> kept alive, so we can safely pass NULL for this third parameter.

The patch looks good and works correctly however from the bug caused
by 8/9 I cannot say that this rationale is currently completely true
(I agree it should be).

Frediano

> ---
>  server/cursor-channel.c        |  6 +++---
>  server/inputs-channel-client.c |  2 +-
>  server/inputs-channel.c        |  6 +++---
>  server/main-channel-client.c   | 28 ++++++++++++++--------------
>  server/smartcard.c             |  2 +-
>  server/spicevmc.c              |  6 +++---
>  6 files changed, 25 insertions(+), 25 deletions(-)
> 
> diff --git a/server/cursor-channel.c b/server/cursor-channel.c
> index 14672ca..dedee37 100644
> --- a/server/cursor-channel.c
> +++ b/server/cursor-channel.c
> @@ -225,7 +225,7 @@ static void cursor_marshall(CursorChannelClient *ccc,
>      case QXL_CURSOR_MOVE:
>          {
>              SpiceMsgCursorMove cursor_move;
> -            red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_MOVE,
> pipe_item);
> +            red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_MOVE,
> NULL);
>              cursor_move.position = cmd->u.position;
>              spice_marshall_msg_cursor_move(m, &cursor_move);
>              break;
> @@ -245,13 +245,13 @@ static void cursor_marshall(CursorChannelClient *ccc,
>              break;
>          }
>      case QXL_CURSOR_HIDE:
> -        red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_HIDE,
> pipe_item);
> +        red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_HIDE, NULL);
>          break;
>      case QXL_CURSOR_TRAIL:
>          {
>              SpiceMsgCursorTrail cursor_trail;
>  
> -            red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_TRAIL,
> pipe_item);
> +            red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_TRAIL,
> NULL);
>              cursor_trail.length = cmd->u.trail.length;
>              cursor_trail.frequency = cmd->u.trail.frequency;
>              spice_marshall_msg_cursor_trail(m, &cursor_trail);
> diff --git a/server/inputs-channel-client.c b/server/inputs-channel-client.c
> index 4ab2457..0200ecd 100644
> --- a/server/inputs-channel-client.c
> +++ b/server/inputs-channel-client.c
> @@ -89,7 +89,7 @@ void
> inputs_channel_client_send_migrate_data(RedChannelClient *rcc,
>  {
>      InputsChannelClient *icc = INPUTS_CHANNEL_CLIENT(rcc);
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, NULL);
>  
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_INPUTS_MAGIC);
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_INPUTS_VERSION);
> diff --git a/server/inputs-channel.c b/server/inputs-channel.c
> index bea0ddf..c069b34 100644
> --- a/server/inputs-channel.c
> +++ b/server/inputs-channel.c
> @@ -249,7 +249,7 @@ static void inputs_channel_send_item(RedChannelClient
> *rcc, RedPipeItem *base)
>          {
>              SpiceMsgInputsKeyModifiers key_modifiers;
>  
> -            red_channel_client_init_send_data(rcc,
> SPICE_MSG_INPUTS_KEY_MODIFIERS, base);
> +            red_channel_client_init_send_data(rcc,
> SPICE_MSG_INPUTS_KEY_MODIFIERS, NULL);
>              key_modifiers.modifiers =
>                  SPICE_UPCAST(RedKeyModifiersPipeItem, base)->modifiers;
>              spice_marshall_msg_inputs_key_modifiers(m, &key_modifiers);
> @@ -259,14 +259,14 @@ static void inputs_channel_send_item(RedChannelClient
> *rcc, RedPipeItem *base)
>          {
>              SpiceMsgInputsInit inputs_init;
>  
> -            red_channel_client_init_send_data(rcc, SPICE_MSG_INPUTS_INIT,
> base);
> +            red_channel_client_init_send_data(rcc, SPICE_MSG_INPUTS_INIT,
> NULL);
>              inputs_init.keyboard_modifiers =
>                  SPICE_UPCAST(RedInputsInitPipeItem, base)->modifiers;
>              spice_marshall_msg_inputs_init(m, &inputs_init);
>              break;
>          }
>          case RED_PIPE_ITEM_MOUSE_MOTION_ACK:
> -            red_channel_client_init_send_data(rcc,
> SPICE_MSG_INPUTS_MOUSE_MOTION_ACK, base);
> +            red_channel_client_init_send_data(rcc,
> SPICE_MSG_INPUTS_MOUSE_MOTION_ACK, NULL);
>              break;
>          case RED_PIPE_ITEM_MIGRATE_DATA:
>              INPUTS_CHANNEL(red_channel_client_get_channel(rcc))->src_during_migrate
>              = FALSE;
> diff --git a/server/main-channel-client.c b/server/main-channel-client.c
> index 7e1b1fc..7e55b24 100644
> --- a/server/main-channel-client.c
> +++ b/server/main-channel-client.c
> @@ -771,7 +771,7 @@ static void
> main_channel_marshall_channels(RedChannelClient *rcc,
>      SpiceMsgChannels* channels_info;
>      RedChannel *channel = red_channel_client_get_channel(rcc);
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST,
> item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST,
> NULL);
>      channels_info = reds_msg_channels_new(red_channel_get_server(channel));
>      spice_marshall_msg_main_channels_list(m, channels_info);
>      free(channels_info);
> @@ -785,7 +785,7 @@ static void main_channel_marshall_ping(RedChannelClient
> *rcc,
>      SpiceMsgPing ping;
>      int size_left = item->size;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_PING, &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PING, NULL);
>      ping.id = main_channel_client_next_ping_id(mcc);
>      ping.timestamp = g_get_monotonic_time();
>      spice_marshall_msg_ping(m, &ping);
> @@ -803,7 +803,7 @@ static void
> main_channel_marshall_mouse_mode(RedChannelClient *rcc,
>  {
>      SpiceMsgMainMouseMode mouse_mode;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MOUSE_MODE,
> &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MOUSE_MODE, NULL);
>      mouse_mode.supported_modes = SPICE_MOUSE_MODE_SERVER;
>      if (item->is_client_mouse_allowed) {
>          mouse_mode.supported_modes |= SPICE_MOUSE_MODE_CLIENT;
> @@ -818,7 +818,7 @@ static void
> main_channel_marshall_agent_disconnected(RedChannelClient *rcc,
>  {
>      SpiceMsgMainAgentDisconnect disconnect;
>  
> -    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_AGENT_DISCONNECTED, item);
> +    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_AGENT_DISCONNECTED, NULL);
>      disconnect.error_code = SPICE_LINK_ERR_OK;
>      spice_marshall_msg_main_agent_disconnected(m, &disconnect);
>  }
> @@ -828,7 +828,7 @@ static void main_channel_marshall_tokens(RedChannelClient
> *rcc,
>  {
>      SpiceMsgMainAgentTokens tokens;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_TOKEN,
> &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_TOKEN,
> NULL);
>      tokens.num_tokens = item->tokens;
>      spice_marshall_msg_main_agent_token(m, &tokens);
>  }
> @@ -858,7 +858,7 @@ static void main_channel_marshall_init(RedChannelClient
> *rcc,
>      SpiceMsgMainInit init; // TODO - remove this copy, make RedInitPipeItem
>      reuse SpiceMsgMainInit
>      RedChannel *channel = red_channel_client_get_channel(rcc);
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_INIT,
> &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_INIT, NULL);
>      init.session_id = item->connection_id;
>      init.display_channels_hint = item->display_channels_hint;
>      init.current_mouse_mode = item->current_mouse_mode;
> @@ -878,7 +878,7 @@ static void main_channel_marshall_notify(RedChannelClient
> *rcc,
>  {
>      SpiceMsgNotify notify;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_NOTIFY, &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_NOTIFY, NULL);
>      notify.time_stamp = spice_get_monotonic_time_ns(); // TODO - move to
>      main_new_notify_item
>      notify.severity = SPICE_NOTIFY_SEVERITY_WARN;
>      notify.visibilty = SPICE_NOTIFY_VISIBILITY_HIGH;
> @@ -911,7 +911,7 @@ static void
> main_channel_marshall_migrate_begin(SpiceMarshaller *m, RedChannelCl
>      RedChannel *channel = red_channel_client_get_channel(rcc);
>      SpiceMsgMainMigrationBegin migrate;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MIGRATE_BEGIN,
> item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MIGRATE_BEGIN,
> NULL);
>      main_channel_fill_migrate_dst_info(MAIN_CHANNEL(channel),
>      &migrate.dst_info);
>      spice_marshall_msg_main_migrate_begin(m, &migrate);
>  }
> @@ -923,7 +923,7 @@ static void
> main_channel_marshall_migrate_begin_seamless(SpiceMarshaller *m,
>      RedChannel *channel = red_channel_client_get_channel(rcc);
>      SpiceMsgMainMigrateBeginSeamless migrate_seamless;
>  
> -    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_MIGRATE_BEGIN_SEAMLESS, item);
> +    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_MIGRATE_BEGIN_SEAMLESS, NULL);
>      main_channel_fill_migrate_dst_info(MAIN_CHANNEL(channel),
>      &migrate_seamless.dst_info);
>      migrate_seamless.src_mig_version = SPICE_MIGRATION_PROTOCOL_VERSION;
>      spice_marshall_msg_main_migrate_begin_seamless(m, &migrate_seamless);
> @@ -935,7 +935,7 @@ static void
> main_channel_marshall_multi_media_time(RedChannelClient *rcc,
>  {
>      SpiceMsgMainMultiMediaTime time_mes;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MULTI_MEDIA_TIME,
> &item->base);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MULTI_MEDIA_TIME,
> NULL);
>      time_mes.time = item->time;
>      spice_marshall_msg_main_multi_media_time(m, &time_mes);
>  }
> @@ -949,7 +949,7 @@ static void
> main_channel_marshall_migrate_switch(SpiceMarshaller *m, RedChannelC
>      const RedsMigSpice *mig_target;
>  
>      spice_printerr("");
> -    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST, item);
> +    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST, NULL);
>      main_ch = MAIN_CHANNEL(channel);
>      mig_target = main_channel_get_migration_target(main_ch);
>      migrate.port = mig_target->port;
> @@ -972,7 +972,7 @@ static void
> main_channel_marshall_agent_connected(SpiceMarshaller *m,
>  {
>      SpiceMsgMainAgentConnectedTokens connected;
>  
> -    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_AGENT_CONNECTED_TOKENS, item);
> +    red_channel_client_init_send_data(rcc,
> SPICE_MSG_MAIN_AGENT_CONNECTED_TOKENS, NULL);
>      connected.num_tokens = REDS_AGENT_WINDOW_SIZE;
>      spice_marshall_msg_main_agent_connected_tokens(m, &connected);
>  }
> @@ -1043,11 +1043,11 @@ void main_channel_client_send_item(RedChannelClient
> *rcc, RedPipeItem *base)
>              main_channel_marshall_migrate_switch(m, rcc, base);
>              break;
>          case RED_PIPE_ITEM_TYPE_MAIN_NAME:
> -            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_NAME,
> base);
> +            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_NAME,
> NULL);
>              spice_marshall_msg_main_name(m, &SPICE_UPCAST(RedNamePipeItem,
>              base)->msg);
>              break;
>          case RED_PIPE_ITEM_TYPE_MAIN_UUID:
> -            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_UUID,
> base);
> +            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_UUID,
> NULL);
>              spice_marshall_msg_main_uuid(m, &SPICE_UPCAST(RedUuidPipeItem,
>              base)->msg);
>              break;
>          case RED_PIPE_ITEM_TYPE_MAIN_AGENT_CONNECTED_TOKENS:
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 1a19fa7..9e0d968 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -419,7 +419,7 @@ static void
> smartcard_channel_send_migrate_data(RedChannelClient *rcc,
>  
>      scc = SMARTCARD_CHANNEL_CLIENT(rcc);
>      dev = smartcard_channel_client_get_char_device(scc);
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, NULL);
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SMARTCARD_MAGIC);
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SMARTCARD_VERSION);
>  
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 765d287..d6a6ac8 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -655,7 +655,7 @@ static void
> spicevmc_red_channel_send_migrate_data(RedChannelClient *rcc,
>      RedVmcChannel *channel;
>  
>      channel = RED_VMC_CHANNEL(red_channel_client_get_channel(rcc));
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, NULL);
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SPICEVMC_MAGIC);
>      spice_marshaller_add_uint32(m, SPICE_MIGRATE_DATA_SPICEVMC_VERSION);
>  
> @@ -669,7 +669,7 @@ static void
> spicevmc_red_channel_send_port_init(RedChannelClient *rcc,
>      RedPortInitPipeItem *i = SPICE_UPCAST(RedPortInitPipeItem, item);
>      SpiceMsgPortInit init;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_INIT, item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_INIT, NULL);
>      init.name = (uint8_t *)i->name;
>      init.name_size = strlen(i->name) + 1;
>      init.opened = i->opened;
> @@ -683,7 +683,7 @@ static void
> spicevmc_red_channel_send_port_event(RedChannelClient *rcc,
>      RedPortEventPipeItem *i = SPICE_UPCAST(RedPortEventPipeItem, item);
>      SpiceMsgPortEvent event;
>  
> -    red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_EVENT, item);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_PORT_EVENT, NULL);
>      event.event = i->event;
>      spice_marshall_msg_port_event(m, &event);
>  }

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]