On Wed, 2017-06-14 at 16:40 +0100, Frediano Ziglio wrote:
> TODO: reuse message code (limit messages based on type ??)
I don't quite understand what you mean above
> document limit on cursor.
or here?
> use finalize instead of dispose for message.
or here
> reuse code to close and destroy channels
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/stream-device.c | 117
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 116 insertions(+), 1 deletion(-)
>
> diff --git a/server/stream-device.c b/server/stream-device.c
> index 2b1e2f2..2513b8c 100644
> --- a/server/stream-device.c
> +++ b/server/stream-device.c
> @@ -23,6 +23,7 @@
>
> #include "char-device.h"
> #include "stream-channel.h"
> +#include "cursor-channel.h"
> #include "reds.h"
>
> #define TYPE_STREAM_DEVICE stream_device_get_type()
> @@ -46,6 +47,9 @@ struct StreamDevice {
> bool opened;
> bool flow_stopped;
> StreamChannel *channel;
> + CursorChannel *cursor_channel;
> + void *msg_buf;
> + uint32_t msg_len;
> };
>
> struct StreamDeviceClass {
> @@ -59,7 +63,8 @@ G_DEFINE_TYPE(StreamDevice, stream_device,
> RED_TYPE_CHAR_DEVICE)
>
> typedef void StreamMsgHandler(StreamDevice *dev,
> SpiceCharDeviceInstance *sin);
>
> -static StreamMsgHandler handle_msg_format, handle_msg_data,
> handle_msg_invalid;
> +static StreamMsgHandler handle_msg_format, handle_msg_data,
> handle_msg_invalid,
> + handle_msg_cursor_set;
>
> static RedPipeItem *
> stream_device_read_msg_from_dev(RedCharDevice *self,
> SpiceCharDeviceInstance *sin)
> @@ -76,6 +81,11 @@ stream_device_read_msg_from_dev(RedCharDevice
> *self, SpiceCharDeviceInstance *si
>
> /* read header */
> while (dev->hdr_pos < sizeof(dev->hdr)) {
> + if (dev->msg_buf) {
> + free(dev->msg_buf);
> + dev->msg_buf = NULL;
> + dev->msg_len = 0;
> + }
> n = sif->read(sin, (uint8_t *) &dev->hdr, sizeof(dev->hdr) -
> dev->hdr_pos);
> if (n <= 0) {
> return NULL;
> @@ -98,6 +108,9 @@ stream_device_read_msg_from_dev(RedCharDevice
> *self, SpiceCharDeviceInstance *si
> case STREAM_TYPE_DATA:
> handle_msg_data(dev, sin);
> break;
> + case STREAM_TYPE_CURSOR_SET:
> + handle_msg_cursor_set(dev, sin);
> + break;
> case STREAM_TYPE_CAPABILITIES:
> /* FIXME */
> default:
> @@ -177,6 +190,87 @@ handle_msg_data(StreamDevice *dev,
> SpiceCharDeviceInstance *sin)
> }
> }
>
> +static RedCursorCmd *
> +stream_msg_cursor_set_to_cursor_cmd(const StreamMsgCursorSet *msg,
> size_t msg_size)
> +{
> + const QXLCursorHeader* cursor_hdr = &msg->cursor_header;
> +
> + RedCursorCmd *cmd = spice_new0(RedCursorCmd, 1);
> + cmd->type = QXL_CURSOR_SET;
> + cmd->u.set.position.x = 0;
> + cmd->u.set.position.y = 0;
Why is the position hard-coded here?
> + cmd->u.set.visible = 1; // TODO
> + SpiceCursor *cursor = &cmd->u.set.shape;
> + cursor->header.unique = GUINT64_FROM_LE(cursor_hdr->unique);
> + cursor->header.type = GUINT16_FROM_LE(cursor_hdr->type);
> + cursor->header.width = GUINT16_FROM_LE(cursor_hdr->width);
> + cursor->header.height = GUINT16_FROM_LE(cursor_hdr->height);
> + cursor->header.hot_spot_x = GUINT16_FROM_LE(cursor_hdr-
> >hot_spot_x);
> + cursor->header.hot_spot_y = GUINT16_FROM_LE(cursor_hdr-
> >hot_spot_y);
> + if (cursor->header.width > 1024 || cursor->header.height > 1024)
> {
> + free(cmd);
> + return NULL;
> + }
> + size_t size_required = cursor->header.width * cursor-
> >header.height;
> + switch (cursor->header.type) {
> + case SPICE_CURSOR_TYPE_ALPHA:
> + size_required *= 32;
> + break;
> + case SPICE_CURSOR_TYPE_COLOR24:
> + size_required *= 25;
> + break;
> + case SPICE_CURSOR_TYPE_COLOR32:
> + size_required *= 33;
> + break;
> + default:
> + free(cmd);
> + return NULL;
> + }
> + size_required = (size_required + 7) / 8;
I think the above line deserves a short comment
> + if (msg_size < sizeof(StreamMsgCursorSet) + size_required) {
> + free(cmd);
> + return NULL;
> + }
> + cursor->data_size = size_required;
> + cursor->data = spice_memdup(msg->data, size_required);
> + return cmd;
> +}
> +
> +static void
> +handle_msg_cursor_set(StreamDevice *dev, SpiceCharDeviceInstance
> *sin)
> +{
> + SpiceCharDeviceInterface *sif =
> spice_char_device_get_interface(sin);
> +
> + // read part of the message till we get all
> + if (!dev->msg_buf) {
> + if (dev->hdr.size >= 1024 * 1024 * 5 || dev->hdr.size <
> sizeof(StreamMsgCursorSet)) {
#define MAX_CURSOR_SIZE (1024 * 1024 * 5) ?
> + handle_msg_invalid(dev, sin);
> + return;
> + }
> + 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;
> + }
> + dev->msg_len += n;
> + if (dev->msg_len != dev->hdr.size) {
> + return;
> + }
> +
> + // 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;
> + }
> + cursor_channel_process_cmd(dev->cursor_channel, cmd);
> +}
> +
> static void
> stream_device_send_msg_to_client(RedCharDevice *self, RedPipeItem
> *msg, RedClient *client)
> {
> @@ -253,11 +347,19 @@ RedCharDevice *
> stream_device_connect(RedsState *reds, SpiceCharDeviceInstance *sin)
> {
> SpiceCharDeviceInterface *sif;
> + ClientCbs client_cbs = { NULL, };
>
> StreamChannel *channel = stream_channel_new(reds);
>
> + CursorChannel *cursor_channel = cursor_channel_new(reds, NULL,
> reds_get_core_interface(reds));
> + client_cbs.connect = (channel_client_connect_proc)
> cursor_channel_connect;
> + client_cbs.migrate = cursor_channel_client_migrate;
> + red_channel_register_client_cbs(RED_CHANNEL(cursor_channel),
> &client_cbs, NULL);
> + reds_register_channel(reds, RED_CHANNEL(cursor_channel));
> +
> StreamDevice *dev = stream_device_new(sin, reds);
> dev->channel = channel;
> + dev->cursor_channel = cursor_channel;
> stream_channel_register_start_cb(channel,
> stream_device_stream_start, dev);
> stream_channel_register_queue_stat_cb(channel,
> stream_device_stream_queue_stat, dev);
>
> @@ -285,6 +387,19 @@ stream_device_dispose(GObject *object)
> red_channel_destroy(red_channel);
> device->channel = NULL;
> }
> + if (device->cursor_channel) {
> + RedChannel *red_channel = RED_CHANNEL(device-
> >cursor_channel);
> + RedsState *reds = red_channel_get_server(red_channel);
> +
> + // prevent future connection
> + reds_unregister_channel(reds, red_channel);
> +
> + // close all current connections and drop the reference
> + red_channel_destroy(red_channel);
> + device->cursor_channel = NULL;
> + }
> + free(device->msg_buf);
> + device->msg_buf = 0;
> }
>
> static void
Looks OK in general.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel