[Spice-devel] [spice-server v2 02/14] channel: Remove RedChannel::handle_parsed
Frediano Ziglio
fziglio at redhat.com
Tue Feb 14 16:07:49 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;
Acked-by: Frediano Ziglio <fziglio at redhat.com>
Frediano
More information about the Spice-devel
mailing list