[Spice-commits] 2 commits - server/cursor-channel.c server/inputs-channel-client.c server/inputs-channel.c server/main-channel-client.c server/red-channel-client.c server/smartcard.c server/spicevmc.c
Frediano Ziglio
fziglio at kemper.freedesktop.org
Tue Dec 20 10:18:01 UTC 2016
server/cursor-channel.c | 6 +++---
server/inputs-channel-client.c | 2 +-
server/inputs-channel.c | 6 +++---
server/main-channel-client.c | 28 ++++++++++++++--------------
server/red-channel-client.c | 7 +++----
server/smartcard.c | 2 +-
server/spicevmc.c | 6 +++---
7 files changed, 28 insertions(+), 29 deletions(-)
New commits:
commit 5c0f2e341c976ce2334e1beb5cd60e07fc00bc8f
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date: Mon Dec 19 15:05:25 2016 -0600
Avoid passing pipe item to red_channel_client_init_send_data()
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.
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index fded226..04483af 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 3781b85..f628f48 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 e51feea..a85c80a 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);
}
commit 0239dfa908b1bc31701c85623f9ae8b05376d103
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date: Mon Dec 19 15:05:24 2016 -0600
Reset marshaller as soon as a message is sent
Any data that is added to the marshaller by reference (using e.g.
spice_marshaller_add_by_ref_full()) is freed during
spice_marshaller_reset(). But the marshaller is not currently reset
until we begin to send the next message (in
red_channel_client_send_item()). This means that the sent message data
lives longer than expected and can violate some assumptions in other
parts of the code.
To make sure that the data is cleaned up right after being sent, I've
added a reset call to clear_sent_item() and called that function from
_on_out_msg_done(). This means that _restore_main_sender() no longer
needs to reset the marshaller, and we no longer need to call
_reset_sent_item() within _on_out_msg_done() (since this function is
called from _clear_sent_item()).
This prepares the way for refactoring
red_channel_client_init_send_data() to change how we keep the
RedPipeItem alive while sending a message.
Acked-by: Frediano Ziglio <fziglio at redhat.com>
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 24b2fb0..a69146a 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -40,6 +40,7 @@
static const SpiceDataHeaderOpaque full_header_wrapper;
static const SpiceDataHeaderOpaque mini_header_wrapper;
+static void red_channel_client_clear_sent_item(RedChannelClient *rcc);
static void red_channel_client_destroy_remote_caps(RedChannelClient* rcc);
static void red_channel_client_initable_interface_init(GInitableIface *iface);
@@ -550,7 +551,6 @@ static inline void red_channel_client_release_sent_item(RedChannelClient *rcc)
static void red_channel_client_restore_main_sender(RedChannelClient *rcc)
{
- spice_marshaller_reset(rcc->priv->send_data.urgent.marshaller);
rcc->priv->send_data.marshaller = rcc->priv->send_data.main.marshaller;
rcc->priv->send_data.header.data = rcc->priv->send_data.main.header_data;
rcc->priv->send_data.item = rcc->priv->send_data.main.item;
@@ -561,8 +561,6 @@ void red_channel_client_on_out_msg_done(void *opaque)
RedChannelClient *rcc = RED_CHANNEL_CLIENT(opaque);
int fd;
- rcc->priv->send_data.size = 0;
-
if (spice_marshaller_get_fd(rcc->priv->send_data.marshaller, &fd)) {
if (reds_stream_send_msgfd(rcc->priv->stream, fd) < 0) {
perror("sendfd");
@@ -575,7 +573,7 @@ void red_channel_client_on_out_msg_done(void *opaque)
close(fd);
}
- red_channel_client_release_sent_item(rcc);
+ red_channel_client_clear_sent_item(rcc);
if (rcc->priv->send_data.blocked) {
SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
rcc->priv->send_data.blocked = FALSE;
@@ -1592,6 +1590,7 @@ static void red_channel_client_clear_sent_item(RedChannelClient *rcc)
red_channel_client_release_sent_item(rcc);
rcc->priv->send_data.blocked = FALSE;
rcc->priv->send_data.size = 0;
+ spice_marshaller_reset(rcc->priv->send_data.marshaller);
}
// TODO: again - what is the context exactly? this happens in channel disconnect. but our
More information about the Spice-commits
mailing list