>
> This is currently unlikely to happen since we communicate over a pipe
> and the pipe buffer is sufficiently large to avoid splitting the
> message. But for completeness, we should handle this scenario.
>
> Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
> ---
> server/stream-device.c | 33 +++++++++++++++++++++++----------
> 1 file changed, 23 insertions(+), 10 deletions(-)
>
> diff --git a/server/stream-device.c b/server/stream-device.c
> index fc5b50659..259527246 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -42,6 +42,12 @@ struct StreamDevice {
>
> StreamDevHeader hdr;
> uint8_t hdr_pos;
> + union {
> + StreamMsgFormat format;
> + StreamMsgCapabilities capabilities;
> + uint8_t buf[1024];
why capabilities ?
why a buffer of 1024 ?
> + } msg;
> + uint8_t msg_pos;
uint8_t is not big enough for the size of msg.
> bool has_error;
> bool opened;
> bool flow_stopped;
> @@ -155,20 +161,27 @@ handle_msg_invalid(StreamDevice *dev,
> SpiceCharDeviceInstance *sin, const char *
> 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 false;
> - }
> - if (n != sizeof(fmt)) {
> +
> + spice_assert(dev->hdr_pos >= sizeof(StreamDevHeader));
> + spice_assert(dev->hdr.type == STREAM_TYPE_FORMAT);
> +
> + int n = sif->read(sin, dev->msg.buf + dev->msg_pos,
> sizeof(StreamMsgFormat) - dev->msg_pos);
> + if (n < 0) {
> return handle_msg_invalid(dev, sin, NULL);
> }
> - fmt.width = GUINT32_FROM_LE(fmt.width);
> - fmt.height = GUINT32_FROM_LE(fmt.height);
> - stream_channel_change_format(dev->stream_channel, &fmt);
>
> - return true;
> + dev->msg_pos += n;
> +
> + if (dev->msg_pos >= sizeof(StreamMsgFormat)) {
I would do a "if (dev->msg_pos < sizeof(StreamMsgFormat)) return false" instead.
> + dev->msg.format.width = GUINT32_FROM_LE(dev->msg.format.width);
> + dev->msg.format.height = GUINT32_FROM_LE(dev->msg.format.height);
> + stream_channel_change_format(dev->stream_channel, &dev->msg.format);
> + dev->msg_pos = 0;
should this reset be in the main code ? I think so.
> + return true;
> + }
> +
> + return false;
> }
>
> static bool
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel