[Spice-devel] [PATCH v1 10/10] Remove third argument from red_channel_client_init_send_data()

Jonathon Jongsma jjongsma at redhat.com
Wed Dec 14 16:36:45 UTC 2016


This third argument (and the 'item' member of
RedChannelClient::priv::send_data) was a somewhat roundabout way to keep
the RedPipeItem alive until a message is sent, just in case some data
owned by that pipeitem was added to the marshaller by reference. This
was a rather confusing mechanism, however, since it did not have any
obvious connection to the _add_by_ref() call. It was never very clear
whether you needed to pass an item to this function or not. The previous
series of patches made this parameter unnecessary since the referencing
of the pipe item (or other related structure) is now more explicitly
connected to the calls to spice_marshaller_add_by_ref_full().
---
 server/inputs-channel-client.c      |  2 +-
 server/inputs-channel.c             |  6 +++---
 server/main-channel-client.c        | 32 ++++++++++++++++----------------
 server/red-channel-client-private.h |  1 -
 server/red-channel-client.c         | 17 +----------------
 server/red-channel-client.h         |  2 +-
 6 files changed, 22 insertions(+), 38 deletions(-)

diff --git a/server/inputs-channel-client.c b/server/inputs-channel-client.c
index 0200ecd..4a534e8 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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA);
 
     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 c069b34..fd29425 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, NULL);
+            red_channel_client_init_send_data(rcc, SPICE_MSG_INPUTS_KEY_MODIFIERS);
             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, NULL);
+            red_channel_client_init_send_data(rcc, SPICE_MSG_INPUTS_INIT);
             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, NULL);
+            red_channel_client_init_send_data(rcc, SPICE_MSG_INPUTS_MOUSE_MOTION_ACK);
             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 4a64a40..fbec09a 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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_CHANNELS_LIST);
     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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_PING);
     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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MOUSE_MODE);
     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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_DISCONNECTED);
     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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_TOKEN);
     tokens.num_tokens = item->tokens;
     spice_marshall_msg_main_agent_token(m, &tokens);
 }
@@ -843,7 +843,7 @@ static void main_channel_marshall_agent_data(RedChannelClient *rcc,
                                              SpiceMarshaller *m,
                                              RedAgentDataPipeItem *item)
 {
-    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_DATA, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_DATA);
     /* since pipe item owns the data, keep it alive until it's sent */
     red_pipe_item_ref(&item->base);
     spice_marshaller_add_by_ref_full(m, item->data, item->len, marshaller_free_pipe_item, item);
@@ -854,7 +854,7 @@ static void main_channel_marshall_migrate_data_item(RedChannelClient *rcc,
                                                     RedPipeItem *item)
 {
     RedChannel *channel = red_channel_client_get_channel(rcc);
-    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MIGRATE_DATA);
     // TODO: from reds split. ugly separation.
     reds_marshall_migrate_data(red_channel_get_server(channel), m);
 }
@@ -866,7 +866,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_INIT);
     init.session_id = item->connection_id;
     init.display_channels_hint = item->display_channels_hint;
     init.current_mouse_mode = item->current_mouse_mode;
@@ -886,7 +886,7 @@ static void main_channel_marshall_notify(RedChannelClient *rcc,
 {
     SpiceMsgNotify notify;
 
-    red_channel_client_init_send_data(rcc, SPICE_MSG_NOTIFY, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_NOTIFY);
     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;
@@ -919,7 +919,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MIGRATE_BEGIN);
     main_channel_fill_migrate_dst_info(MAIN_CHANNEL(channel), &migrate.dst_info);
     spice_marshall_msg_main_migrate_begin(m, &migrate);
 }
@@ -931,7 +931,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MIGRATE_BEGIN_SEAMLESS);
     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);
@@ -943,7 +943,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MULTI_MEDIA_TIME);
     time_mes.time = item->time;
     spice_marshall_msg_main_multi_media_time(m, &time_mes);
 }
@@ -957,7 +957,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_MIGRATE_SWITCH_HOST);
     main_ch = MAIN_CHANNEL(channel);
     mig_target = main_channel_get_migration_target(main_ch);
     migrate.port = mig_target->port;
@@ -980,7 +980,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, NULL);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_AGENT_CONNECTED_TOKENS);
     connected.num_tokens = REDS_AGENT_WINDOW_SIZE;
     spice_marshall_msg_main_agent_connected_tokens(m, &connected);
 }
@@ -1051,11 +1051,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, NULL);
+            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_NAME);
             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, NULL);
+            red_channel_client_init_send_data(rcc, SPICE_MSG_MAIN_UUID);
             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/red-channel-client-private.h b/server/red-channel-client-private.h
index 0cb3ddd..76c9574 100644
--- a/server/red-channel-client-private.h
+++ b/server/red-channel-client-private.h
@@ -69,7 +69,6 @@ struct RedChannelClientPrivate
         SpiceMarshaller *marshaller;
         SpiceDataHeaderOpaque header;
         uint32_t size;
-        RedPipeItem *item;
         int blocked;
         uint64_t last_sent_serial;
 
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 24b2fb0..e543c27 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -540,20 +540,11 @@ static void red_channel_client_send_item(RedChannelClient *rcc, RedPipeItem *ite
     red_pipe_item_unref(item);
 }
 
-static inline void red_channel_client_release_sent_item(RedChannelClient *rcc)
-{
-    if (rcc->priv->send_data.item) {
-        red_pipe_item_unref(rcc->priv->send_data.item);
-        rcc->priv->send_data.item = NULL;
-    }
-}
-
 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;
 }
 
 void red_channel_client_on_out_msg_done(void *opaque)
@@ -575,7 +566,6 @@ void red_channel_client_on_out_msg_done(void *opaque)
             close(fd);
     }
 
-    red_channel_client_release_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;
@@ -1393,15 +1383,11 @@ int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
     return TRUE;
 }
 
-void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type, RedPipeItem *item)
+void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type)
 {
     spice_assert(red_channel_client_no_item_being_sent(rcc));
     spice_assert(msg_type != 0);
     rcc->priv->send_data.header.set_msg_type(&rcc->priv->send_data.header, msg_type);
-    rcc->priv->send_data.item = item;
-    if (item) {
-        red_pipe_item_ref(item);
-    }
 }
 
 void red_channel_client_begin_send_message(RedChannelClient *rcc)
@@ -1589,7 +1575,6 @@ gboolean red_channel_client_is_connected(RedChannelClient *rcc)
 
 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;
 }
diff --git a/server/red-channel-client.h b/server/red-channel-client.h
index 94b4f58..0d404d1 100644
--- a/server/red-channel-client.h
+++ b/server/red-channel-client.h
@@ -89,7 +89,7 @@ void red_channel_client_shutdown(RedChannelClient *rcc);
 int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
                                       uint16_t type, void *message);
 /* when preparing send_data: should call init and then use marshaller */
-void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type, RedPipeItem *item);
+void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type);
 
 uint64_t red_channel_client_get_message_serial(RedChannelClient *channel);
 void red_channel_client_set_message_serial(RedChannelClient *channel, uint64_t);
-- 
2.7.4



More information about the Spice-devel mailing list