[Spice-devel] [PATCH spice-server] StreamDevice: move header reset to calling function
Jonathon Jongsma
jjongsma at redhat.com
Tue Aug 29 14:31:24 UTC 2017
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
More information about the Spice-devel
mailing list