[Spice-devel] [PATCH v2 1/9] Avoid passing pipe item to red_channel_client_init_send_data()
Frediano Ziglio
fziglio at redhat.com
Fri Dec 16 15:31:33 UTC 2016
> 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);
> }
More information about the Spice-devel
mailing list