[Spice-devel] [spice-server v2 02/14] channel: Remove RedChannel::handle_parsed

Christophe Fergeau cfergeau at redhat.com
Tue Feb 14 14:17:27 UTC 2017


red_channel_client_parse() currently does roughly:

if (klass->parser) {
    parsed = klass->parser(msg, msg_size, &parsed_size);
    klass->handle_parsed(rcc, parsed_size, msg_type, parsed);
} else {
    klass->handle_message(rcc, msg_type, msg, msg_size);
}

The handle_parsed implementation expects a void * 'parsed' argument,
which will then be cast to the correct type. There is not really a need
to provide distinct handle_parsed/handle_message vfuncs, instead we can
say that if a RedChannel subclass provides a 'parser' vfunc, then it's
'handle_message' vfunc will be called with the parsed message, otherwise
it will be called with unparsed data (ie what handle_message currently
expects).

This makes the code slightly easier to follow as messages will always be
handled by the same vfunc.

Signed-off-by: Christophe Fergeau <cfergeau at redhat.com>
---
 server/cursor-channel.c           |  2 +-
 server/dcc.c                      |  4 +--
 server/dcc.h                      |  4 +--
 server/display-channel.c          |  2 +-
 server/inputs-channel.c           |  8 +++---
 server/main-channel-client.c      |  2 +-
 server/main-channel.c             |  8 +++---
 server/red-channel-client.c       | 57 ++++++++++++++++++++++++---------------
 server/red-channel-client.h       |  4 +--
 server/red-channel.c              |  1 -
 server/red-channel.h              | 23 ++++++++--------
 server/smartcard-channel-client.c |  7 ++---
 server/smartcard-channel-client.h |  2 +-
 server/sound.c                    |  8 +++---
 server/spicevmc.c                 | 12 ++++-----
 15 files changed, 79 insertions(+), 65 deletions(-)

diff --git a/server/cursor-channel.c b/server/cursor-channel.c
index b3cbb96..4fe3f8d 100644
--- a/server/cursor-channel.c
+++ b/server/cursor-channel.c
@@ -448,7 +448,7 @@ cursor_channel_class_init(CursorChannelClass *klass)
     object_class->finalize = cursor_channel_finalize;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_CURSOR, NULL);
-    channel_class->handle_parsed = red_channel_client_handle_message;
+    channel_class->handle_message = red_channel_client_handle_message;
 
     channel_class->on_disconnect =  cursor_channel_client_on_disconnect;
     channel_class->send_item = cursor_channel_send_item;
diff --git a/server/dcc.c b/server/dcc.c
index afe37b1..cf9431a 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -1121,7 +1121,7 @@ static int dcc_handle_gl_draw_done(DisplayChannelClient *dcc)
     return TRUE;
 }
 
-int dcc_handle_message(RedChannelClient *rcc, uint32_t size, uint16_t type, void *msg)
+int dcc_handle_message(RedChannelClient *rcc, uint16_t type, uint32_t size, void *msg)
 {
     DisplayChannelClient *dcc = DISPLAY_CHANNEL_CLIENT(rcc);
 
@@ -1136,7 +1136,7 @@ int dcc_handle_message(RedChannelClient *rcc, uint32_t size, uint16_t type, void
     case SPICE_MSGC_DISPLAY_GL_DRAW_DONE:
         return dcc_handle_gl_draw_done(dcc);
     default:
-        return red_channel_client_handle_message(rcc, size, type, msg);
+        return red_channel_client_handle_message(rcc, type, size, msg);
     }
 }
 
diff --git a/server/dcc.h b/server/dcc.h
index 6fb468d..ebdbb8d 100644
--- a/server/dcc.h
+++ b/server/dcc.h
@@ -144,8 +144,8 @@ DisplayChannelClient*      dcc_new                                   (DisplayCha
 void                       dcc_start                                 (DisplayChannelClient *dcc);
 void                       dcc_stop                                  (DisplayChannelClient *dcc);
 int                        dcc_handle_message                        (RedChannelClient *rcc,
-                                                                      uint32_t size,
-                                                                      uint16_t type, void *msg);
+                                                                      uint16_t type,
+                                                                      uint32_t size, void *msg);
 int                        dcc_handle_migrate_data                   (DisplayChannelClient *dcc,
                                                                       uint32_t size, void *message);
 void                       dcc_push_monitors_config                  (DisplayChannelClient *dcc);
diff --git a/server/display-channel.c b/server/display-channel.c
index 7d3c6e4..9a9385f 100644
--- a/server/display-channel.c
+++ b/server/display-channel.c
@@ -2229,7 +2229,7 @@ display_channel_class_init(DisplayChannelClass *klass)
     object_class->finalize = display_channel_finalize;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_DISPLAY, NULL);
-    channel_class->handle_parsed = dcc_handle_message;
+    channel_class->handle_message = dcc_handle_message;
 
     channel_class->on_disconnect = on_disconnect;
     channel_class->send_item = dcc_send_item;
diff --git a/server/inputs-channel.c b/server/inputs-channel.c
index e197f68..ed490e9 100644
--- a/server/inputs-channel.c
+++ b/server/inputs-channel.c
@@ -279,8 +279,8 @@ static void inputs_channel_send_item(RedChannelClient *rcc, RedPipeItem *base)
     red_channel_client_begin_send_message(rcc);
 }
 
-static int inputs_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type,
-                                        void *message)
+static int inputs_channel_handle_message(RedChannelClient *rcc, uint16_t type,
+                                         uint32_t size, void *message)
 {
     InputsChannel *inputs_channel = INPUTS_CHANNEL(red_channel_client_get_channel(rcc));
     InputsChannelClient *icc = INPUTS_CHANNEL_CLIENT(rcc);
@@ -436,7 +436,7 @@ static int inputs_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, ui
     case SPICE_MSGC_DISCONNECTING:
         break;
     default:
-        return red_channel_client_handle_message(rcc, size, type, message);
+        return red_channel_client_handle_message(rcc, type, size, message);
     }
     return TRUE;
 }
@@ -646,7 +646,7 @@ inputs_channel_class_init(InputsChannelClass *klass)
     object_class->finalize = inputs_channel_finalize;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_INPUTS, NULL);
-    channel_class->handle_parsed = inputs_channel_handle_parsed;
+    channel_class->handle_message = inputs_channel_handle_message;
 
     /* channel callbacks */
     channel_class->config_socket = inputs_channel_config_socket;
diff --git a/server/main-channel-client.c b/server/main-channel-client.c
index 94150e6..6aace29 100644
--- a/server/main-channel-client.c
+++ b/server/main-channel-client.c
@@ -486,7 +486,7 @@ void main_channel_client_handle_pong(MainChannelClient *mcc, SpiceMsgPing *ping,
         /*
          * channel client monitors the connectivity using ping-pong messages
          */
-        red_channel_client_handle_message(rcc, size, SPICE_MSGC_PONG, ping);
+        red_channel_client_handle_message(rcc, SPICE_MSGC_PONG, size, ping);
         return;
     }
 
diff --git a/server/main-channel.c b/server/main-channel.c
index 745f155..fceefa2 100644
--- a/server/main-channel.c
+++ b/server/main-channel.c
@@ -179,8 +179,8 @@ void main_channel_migrate_switch(MainChannel *main_chan, RedsMigSpice *mig_targe
     red_channel_pipes_add_type(RED_CHANNEL(main_chan), RED_PIPE_ITEM_TYPE_MAIN_MIGRATE_SWITCH_HOST);
 }
 
-static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type,
-                                      void *message)
+static int main_channel_handle_message(RedChannelClient *rcc, uint16_t type,
+                                       uint32_t size, void *message)
 {
     RedChannel *channel = red_channel_client_get_channel(rcc);
     MainChannel *main_chan = MAIN_CHANNEL(channel);
@@ -243,7 +243,7 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
         main_channel_client_handle_migrate_end(mcc);
         break;
     default:
-        return red_channel_client_handle_message(rcc, size, type, message);
+        return red_channel_client_handle_message(rcc, type, size, message);
     }
     return TRUE;
 }
@@ -352,7 +352,7 @@ main_channel_class_init(MainChannelClass *klass)
     object_class->constructed = main_channel_constructed;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_MAIN, NULL);
-    channel_class->handle_parsed = main_channel_handle_parsed;
+    channel_class->handle_message = main_channel_handle_message;
 
     /* channel callbacks */
     channel_class->config_socket = main_channel_config_socket;
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 06fb8a8..52dee70 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -1082,6 +1082,24 @@ static int red_peer_receive(RedsStream *stream, uint8_t *buf, uint32_t size)
     return pos - buf;
 }
 
+static uint8_t *red_channel_client_parse(IncomingHandler *handler, uint8_t *message, size_t message_size,
+                                         uint16_t message_type,
+                                         size_t *size_out, message_destructor_t *free_message)
+{
+    uint8_t *parsed_message;
+
+    if (handler->cb->parser) {
+        parsed_message = handler->cb->parser(message, message + message_size, message_type,
+                                             SPICE_VERSION_MINOR, size_out, free_message);
+    } else {
+        parsed_message = message;
+        *size_out = message_size;
+        *free_message = NULL;
+    }
+
+    return parsed_message;
+}
+
 // TODO: this implementation, as opposed to the old implementation in red_worker,
 // does many calls to red_peer_receive and through it cb_read, and thus avoids pointer
 // arithmetic for the case where a single cb_read could return multiple messages. But
@@ -1100,6 +1118,9 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
 
     for (;;) {
         int ret_handle;
+        uint8_t *parsed;
+        size_t parsed_size;
+        message_destructor_t parsed_free = NULL;
         if (handler->header_pos < handler->header.header_size) {
             bytes_read = red_peer_receive(stream,
                                           handler->header.data + handler->header_pos,
@@ -1143,26 +1164,20 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
             }
         }
 
-        if (handler->cb->parser) {
-            uint8_t *parsed;
-            size_t parsed_size;
-            message_destructor_t parsed_free;
-
-            parsed = handler->cb->parser(handler->msg,
-                handler->msg + msg_size, msg_type,
-                SPICE_VERSION_MINOR, &parsed_size, &parsed_free);
-            if (parsed == NULL) {
-                spice_printerr("failed to parse message type %d", msg_type);
-                handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
-                handler->cb->on_error(handler->opaque);
-                return;
-            }
-            ret_handle = handler->cb->handle_parsed(handler->opaque, parsed_size,
-                                                    msg_type, parsed);
+        parsed = red_channel_client_parse(handler,
+                                          handler->msg, msg_size,
+                                          msg_type,
+                                          &parsed_size, &parsed_free);
+        if (parsed == NULL) {
+            spice_printerr("failed to parse message type %d", msg_type);
+            handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
+            handler->cb->on_error(handler->opaque);
+            return;
+        }
+        ret_handle = handler->cb->handle_message(handler->opaque, msg_type,
+                                                 parsed_size, parsed);
+        if (parsed_free != NULL) {
             parsed_free(parsed);
-        } else {
-            ret_handle = handler->cb->handle_message(handler->opaque, msg_type, msg_size,
-                                                     handler->msg);
         }
         handler->msg_pos = 0;
         handler->cb->release_msg_buf(handler->opaque, msg_type, msg_size, handler->msg);
@@ -1343,8 +1358,8 @@ static void red_channel_client_handle_migrate_data(RedChannelClient *rcc,
 }
 
 
-int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
-                                      uint16_t type, void *message)
+int red_channel_client_handle_message(RedChannelClient *rcc, uint16_t type,
+                                      uint32_t size, void *message)
 {
     switch (type) {
     case SPICE_MSGC_ACK_SYNC:
diff --git a/server/red-channel-client.h b/server/red-channel-client.h
index fada609..d54e7dd 100644
--- a/server/red-channel-client.h
+++ b/server/red-channel-client.h
@@ -86,8 +86,8 @@ int red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap);
  * It should be followed by some way to guarantee a disconnection. */
 void red_channel_client_shutdown(RedChannelClient *rcc);
 /* handles general channel msgs from the client */
-int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
-                                      uint16_t type, void *message);
+int red_channel_client_handle_message(RedChannelClient *rcc, uint16_t type,
+                                      uint32_t size, 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);
 
diff --git a/server/red-channel.c b/server/red-channel.c
index 835a744..9d492e6 100644
--- a/server/red-channel.c
+++ b/server/red-channel.c
@@ -235,7 +235,6 @@ red_channel_constructed(GObject *object)
     self->priv->incoming_cb.release_msg_buf =
         (release_msg_recv_buf_proc)klass->release_recv_buf;
     self->priv->incoming_cb.handle_message = (handle_message_proc)klass->handle_message;
-    self->priv->incoming_cb.handle_parsed = (handle_parsed_proc)klass->handle_parsed;
     self->priv->incoming_cb.parser = klass->parser;
 }
 
diff --git a/server/red-channel.h b/server/red-channel.h
index 3392560..0cdf564 100644
--- a/server/red-channel.h
+++ b/server/red-channel.h
@@ -61,7 +61,6 @@ struct SpiceDataHeaderOpaque {
 
 typedef int (*handle_message_proc)(void *opaque,
                                    uint16_t type, uint32_t size, uint8_t *msg);
-typedef int (*handle_parsed_proc)(void *opaque, uint32_t size, uint16_t type, void *message);
 typedef uint8_t *(*alloc_msg_recv_buf_proc)(void *opaque, uint16_t type, uint32_t size);
 typedef void (*release_msg_recv_buf_proc)(void *opaque,
                                           uint16_t type, uint32_t size, uint8_t *msg);
@@ -69,13 +68,12 @@ typedef void (*on_incoming_error_proc)(void *opaque);
 typedef void (*on_input_proc)(void *opaque, int n);
 
 typedef struct IncomingHandlerInterface {
-    handle_message_proc handle_message;
     alloc_msg_recv_buf_proc alloc_msg_buf;
     on_incoming_error_proc on_error; // recv error or handle_message error
     release_msg_recv_buf_proc release_msg_buf; // for errors
-    // The following is an optional alternative to handle_message, used if not null
+    // 'parser' is optional and will not be used if NULL
     spice_parse_channel_func_t parser;
-    handle_parsed_proc handle_parsed;
+    handle_message_proc handle_message;
     on_input_proc on_input;
 } IncomingHandlerInterface;
 
@@ -103,10 +101,8 @@ typedef struct MainChannelClient MainChannelClient;
 
 typedef uint8_t *(*channel_alloc_msg_recv_buf_proc)(RedChannelClient *channel,
                                                     uint16_t type, uint32_t size);
-typedef int (*channel_handle_parsed_proc)(RedChannelClient *rcc, uint32_t size, uint16_t type,
-                                        void *message);
-typedef int (*channel_handle_message_proc)(RedChannelClient *rcc,
-                                           uint16_t type, uint32_t size, uint8_t *msg);
+typedef int (*channel_handle_message_proc)(RedChannelClient *rcc, uint16_t type,
+                                           uint32_t size, void *msg);
 typedef void (*channel_release_msg_recv_buf_proc)(RedChannelClient *channel,
                                                   uint16_t type, uint32_t size, uint8_t *msg);
 typedef void (*channel_disconnect_proc)(RedChannelClient *rcc);
@@ -166,11 +162,14 @@ struct RedChannelClass
 {
     GObjectClass parent_class;
 
-    /* subclasses must implement either handle_message(), or both parser() and
-     * handle_parsed() */
+    /* subclasses must implement handle_message() and optionally parser().
+     * If parser() is implemented, then handle_message() will get passed the
+     * parsed message as its 'msg' argument, otherwise it will be passed
+     * the raw data. In both cases, the 'size' argument is the length of 'msg'
+     * in bytes
+     */
+    spice_parse_channel_func_t parser;
     channel_handle_message_proc handle_message;
-    spice_parse_channel_func_t parser;
-    channel_handle_parsed_proc handle_parsed;
 
     // TODO: add ASSERTS for thread_id  in client and channel calls
     /*
diff --git a/server/smartcard-channel-client.c b/server/smartcard-channel-client.c
index 668a1fb..6234844 100644
--- a/server/smartcard-channel-client.c
+++ b/server/smartcard-channel-client.c
@@ -295,15 +295,16 @@ static void smartcard_channel_client_write_to_reader(SmartCardChannelClient *scc
 int smartcard_channel_client_handle_message(RedChannelClient *rcc,
                                             uint16_t type,
                                             uint32_t size,
-                                            uint8_t *msg)
+                                            void *message)
 {
-    VSCMsgHeader* vheader = (VSCMsgHeader*)msg;
+    uint8_t *msg = message;
+    VSCMsgHeader* vheader = message;
     SmartCardChannelClient *scc = SMARTCARD_CHANNEL_CLIENT(rcc);
 
     if (type != SPICE_MSGC_SMARTCARD_DATA) {
         /* Handles seamless migration protocol. Also handles ack's,
          * spicy sends them while spicec does not */
-        return red_channel_client_handle_message(rcc, size, type, msg);
+        return red_channel_client_handle_message(rcc, type, size, msg);
     }
 
     spice_assert(size == vheader->length + sizeof(VSCMsgHeader));
diff --git a/server/smartcard-channel-client.h b/server/smartcard-channel-client.h
index db29e20..9350d7a 100644
--- a/server/smartcard-channel-client.h
+++ b/server/smartcard-channel-client.h
@@ -86,7 +86,7 @@ void smartcard_channel_client_send_error(RedChannelClient *rcc,
 int smartcard_channel_client_handle_message(RedChannelClient *rcc,
                                             uint16_t type,
                                             uint32_t size,
-                                            uint8_t *msg);
+                                            void *msg);
 
 int smartcard_channel_client_handle_migrate_data(RedChannelClient *rcc,
                                                  uint32_t size,
diff --git a/server/sound.c b/server/sound.c
index 2692fe5..889ae60 100644
--- a/server/sound.c
+++ b/server/sound.c
@@ -320,7 +320,7 @@ static int snd_record_handle_write(RecordChannelClient *record_client, size_t si
 }
 
 static int
-record_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type, void *message)
+record_channel_handle_message(RedChannelClient *rcc, uint16_t type, uint32_t size, void *message)
 {
     RecordChannelClient *record_client = RECORD_CHANNEL_CLIENT(rcc);
 
@@ -357,7 +357,7 @@ record_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint16_t type
         break;
     }
     default:
-        return red_channel_client_handle_message(rcc, size, type, message);
+        return red_channel_client_handle_message(rcc, type, size, message);
     }
     return TRUE;
 }
@@ -1413,7 +1413,7 @@ playback_channel_class_init(PlaybackChannelClass *klass)
     object_class->constructed = playback_channel_constructed;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_PLAYBACK, NULL);
-    channel_class->handle_parsed = red_channel_client_handle_message;
+    channel_class->handle_message = red_channel_client_handle_message;
     channel_class->send_item = playback_channel_send_item;
 }
 
@@ -1463,7 +1463,7 @@ record_channel_class_init(RecordChannelClass *klass)
     object_class->constructed = record_channel_constructed;
 
     channel_class->parser = spice_get_client_channel_parser(SPICE_CHANNEL_RECORD, NULL);
-    channel_class->handle_parsed = record_channel_handle_parsed;
+    channel_class->handle_message = record_channel_handle_message;
     channel_class->send_item = record_channel_send_item;
 }
 
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 180d4eb..ca7e312 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -550,10 +550,10 @@ static int handle_compressed_msg(RedVmcChannel *channel, RedChannelClient *rcc,
     return TRUE;
 }
 
-static int spicevmc_red_channel_client_handle_message_parsed(RedChannelClient *rcc,
-                                                             uint32_t size,
-                                                             uint16_t type,
-                                                             void *msg)
+static int spicevmc_red_channel_client_handle_message(RedChannelClient *rcc,
+                                                      uint16_t type,
+                                                      uint32_t size,
+                                                      void *msg)
 {
     /* NOTE: *msg free by free() (when cb to spicevmc_red_channel_release_msg_rcv_buf
      * with the compressed msg type) */
@@ -582,7 +582,7 @@ static int spicevmc_red_channel_client_handle_message_parsed(RedChannelClient *r
             sif->event(channel->chardev_sin, *(uint8_t*)msg);
         break;
     default:
-        return red_channel_client_handle_message(rcc, size, type, msg);
+        return red_channel_client_handle_message(rcc, type, size, msg);
     }
 
     return TRUE;
@@ -732,7 +732,7 @@ red_vmc_channel_class_init(RedVmcChannelClass *klass)
     object_class->constructed = red_vmc_channel_constructed;
     object_class->finalize = red_vmc_channel_finalize;
 
-    channel_class->handle_parsed = spicevmc_red_channel_client_handle_message_parsed;
+    channel_class->handle_message = spicevmc_red_channel_client_handle_message;
 
     channel_class->config_socket = spicevmc_red_channel_client_config_socket;
     channel_class->on_disconnect = spicevmc_red_channel_client_on_disconnect;
-- 
2.9.3



More information about the Spice-devel mailing list