Right now, each handle_msg_*() function is required to reset the header parsing state after it is done handling the message. Although the code duplication is pretty minimal (just resetting hdr_pos to 0), it seems a bit cleaner to have the calling function (stream_device_read_msg_from_dev()) handle this. Change the message handler functions to return a boolean value that signals whether they've fully handled the message or not. If the return value is true, the calling function will reset the message parsing state so that we're prepared to parse the next mesage. --- Proposed addition to Frediano's patch series. (Also somewhat related to Christophe D's question about resetting hdr_pos to 0 in patch 22) server/stream-device.c | 69 +++++++++++++++++++++++++++----------------------- 1 file changed, 37 insertions(+), 32 deletions(-) diff --git a/server/stream-device.c b/server/stream-device.c index 3d33ca13d..a0383ae3e 100644 --- a/server/stream-device.c +++ b/server/stream-device.c @@ -61,7 +61,7 @@ static StreamDevice *stream_device_new(SpiceCharDeviceInstance *sin, RedsState * G_DEFINE_TYPE(StreamDevice, stream_device, RED_TYPE_CHAR_DEVICE) -typedef void StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin); +typedef bool StreamMsgHandler(StreamDevice *dev, SpiceCharDeviceInstance *sin); static StreamMsgHandler handle_msg_format, handle_msg_data, handle_msg_invalid, handle_msg_cursor_set, handle_msg_cursor_move; @@ -72,6 +72,7 @@ stream_device_read_msg_from_dev(RedCharDevice *self, SpiceCharDeviceInstance *si StreamDevice *dev = STREAM_DEVICE(self); SpiceCharDeviceInterface *sif; int n; + bool handled = false; if (dev->has_error || dev->flow_stopped || !dev->stream_channel) { return NULL; @@ -100,35 +101,41 @@ stream_device_read_msg_from_dev(RedCharDevice *self, SpiceCharDeviceInstance *si switch ((StreamMsgType) dev->hdr.type) { case STREAM_TYPE_FORMAT: if (dev->hdr.size != sizeof(StreamMsgFormat)) { - handle_msg_invalid(dev, sin); + handled = handle_msg_invalid(dev, sin); } else { - handle_msg_format(dev, sin); + handled = handle_msg_format(dev, sin); } break; case STREAM_TYPE_DATA: - handle_msg_data(dev, sin); + handled = handle_msg_data(dev, sin); break; case STREAM_TYPE_CURSOR_SET: - handle_msg_cursor_set(dev, sin); + handled = handle_msg_cursor_set(dev, sin); break; case STREAM_TYPE_CURSOR_MOVE: if (dev->hdr.size != sizeof(StreamMsgCursorMove)) { - handle_msg_invalid(dev, sin); + handled = handle_msg_invalid(dev, sin); } else { - handle_msg_cursor_move(dev, sin); + handled = handle_msg_cursor_move(dev, sin); } break; case STREAM_TYPE_CAPABILITIES: /* FIXME */ default: - handle_msg_invalid(dev, sin); + handled = handle_msg_invalid(dev, sin); break; } + /* current message has been handled, so reset state and get ready to parse + * the next message */ + if (handled) { + dev->hdr_pos = 0; + } + return NULL; } -static void +static bool handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin) { static const char error_msg[] = "Wrong protocol"; @@ -154,28 +161,29 @@ handle_msg_invalid(StreamDevice *dev, SpiceCharDeviceInstance *sin) red_char_device_write_buffer_add(char_dev, buf); dev->has_error = true; + return true; } -static void +static bool handle_msg_format(StreamDevice *dev, SpiceCharDeviceInstance *sin) { StreamMsgFormat fmt; SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin); int n = sif->read(sin, (uint8_t *) &fmt, sizeof(fmt)); if (n == 0) { - return; + return false; } if (n != sizeof(fmt)) { - handle_msg_invalid(dev, sin); - return; + return handle_msg_invalid(dev, sin); } fmt.width = GUINT32_FROM_LE(fmt.width); fmt.height = GUINT32_FROM_LE(fmt.height); stream_channel_change_format(dev->stream_channel, &fmt); - dev->hdr_pos = 0; + + return true; } -static void +static bool handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin) { SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin); @@ -193,8 +201,10 @@ handle_msg_data(StreamDevice *dev, SpiceCharDeviceInstance *sin) dev->hdr.size -= n; } if (dev->hdr.size == 0) { - dev->hdr_pos = 0; + return true; } + + return false; } /* @@ -262,7 +272,7 @@ stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg, size_t msg_si return cmd; } -static void +static bool handle_msg_cursor_set(StreamDevice *dev, SpiceCharDeviceInstance *sin) { const unsigned int max_cursor_set_size = @@ -273,45 +283,41 @@ handle_msg_cursor_set(StreamDevice *dev, SpiceCharDeviceInstance *sin) // read part of the message till we get all if (!dev->msg_buf) { if (dev->hdr.size >= max_cursor_set_size || dev->hdr.size < sizeof(StreamMsgCursorSet)) { - handle_msg_invalid(dev, sin); - return; + return handle_msg_invalid(dev, sin); } dev->msg_buf = spice_malloc(dev->hdr.size); dev->msg_len = 0; } int n = sif->read(sin, (uint8_t *) dev->msg_buf + dev->msg_len, dev->hdr.size - dev->msg_len); if (n <= 0) { - return; + return false; } dev->msg_len += n; if (dev->msg_len != dev->hdr.size) { - return; + return false; } - // got the full message, prepare for future message - dev->hdr_pos = 0; - // trasform the message to a cursor command and process it RedCursorCmd *cmd = stream_msg_cursor_set_to_cursor_cmd(dev->msg_buf, dev->msg_len); if (!cmd) { - handle_msg_invalid(dev, sin); - return; + return handle_msg_invalid(dev, sin); } cursor_channel_process_cmd(dev->cursor_channel, cmd); + + return true; } -static void +static bool handle_msg_cursor_move(StreamDevice *dev, SpiceCharDeviceInstance *sin) { StreamMsgCursorMove move; SpiceCharDeviceInterface *sif = spice_char_device_get_interface(sin); int n = sif->read(sin, (uint8_t *) &move, sizeof(move)); if (n == 0) { - return; + return false; } if (n != sizeof(move)) { - handle_msg_invalid(dev, sin); - return; + return handle_msg_invalid(dev, sin); } move.x = GINT32_FROM_LE(move.x); move.y = GINT32_FROM_LE(move.y); @@ -323,7 +329,7 @@ handle_msg_cursor_move(StreamDevice *dev, SpiceCharDeviceInstance *sin) cursor_channel_process_cmd(dev->cursor_channel, cmd); - dev->hdr_pos = 0; + return true; } static void @@ -482,7 +488,6 @@ stream_device_port_event(RedCharDevice *char_dev, uint8_t event) if (device->opened) { allocate_channels(device); } - device->hdr_pos = 0; device->has_error = false; device->flow_stopped = false; red_char_device_reset(char_dev); -- 2.13.3 _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel