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@xxxxxxxxxx> --- 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 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel