As a fast look seems that this patch is not updated considering the encapsulation work on CursorChannel. > > --- > server/Makefile.am | 2 + > server/cursor-channel-client.c | 120 > +++++++++++++++++++++++++++++++++++++++++ > server/cursor-channel-client.h | 50 +++++++++++++++++ > server/cursor-channel.c | 101 ++++------------------------------ > server/cursor-channel.h | 12 +---- > server/red-worker.c | 1 + > 6 files changed, 186 insertions(+), 100 deletions(-) > create mode 100644 server/cursor-channel-client.c > create mode 100644 server/cursor-channel-client.h > > diff --git a/server/Makefile.am b/server/Makefile.am > index a249046..e1a6b43 100644 > --- a/server/Makefile.am > +++ b/server/Makefile.am > @@ -118,6 +118,8 @@ libserver_la_SOURCES = \ > red-worker.h \ > display-channel.c \ > display-channel.h \ > + cursor-channel-client.c \ > + cursor-channel-client.h \ > cursor-channel.c \ > cursor-channel.h \ > red-pipe-item.c \ > diff --git a/server/cursor-channel-client.c b/server/cursor-channel-client.c > new file mode 100644 > index 0000000..7dc936b > --- /dev/null > +++ b/server/cursor-channel-client.c > @@ -0,0 +1,120 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2009 Red Hat, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see > <http://www.gnu.org/licenses/>. > +*/ > +#ifdef HAVE_CONFIG_H > +#include <config.h> > +#endif > + > +#include <common/generated_server_marshallers.h> > + > +#include "red-channel-client-private.h" This "private" header is included a bit too much, but is not responsibility of this patch to fix it. > +#include "cache-item.h" > +#include "cursor-channel.h" > + > +#define CLIENT_CURSOR_CACHE_SIZE 256 > + > +#define CURSOR_CACHE_HASH_SHIFT 8 > +#define CURSOR_CACHE_HASH_SIZE (1 << CURSOR_CACHE_HASH_SHIFT) > +#define CURSOR_CACHE_HASH_MASK (CURSOR_CACHE_HASH_SIZE - 1) > +#define CURSOR_CACHE_HASH_KEY(id) ((id) & CURSOR_CACHE_HASH_MASK) > +#define CURSOR_CLIENT_TIMEOUT 30000000000ULL //nano > + > +enum { > + RED_PIPE_ITEM_TYPE_CURSOR = RED_PIPE_ITEM_TYPE_COMMON_LAST, > + RED_PIPE_ITEM_TYPE_CURSOR_INIT, > + RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE, > +}; > + > +struct CursorChannelClient { > + RedChannelClient base; > + > + RedCacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE]; > + Ring cursor_cache_lru; > + long cursor_cache_available; > + uint32_t cursor_cache_items; > +}; > + > + > +#define RCC_TO_CCC(rcc) ((CursorChannelClient*)rcc) > + > +#define CLIENT_CURSOR_CACHE > +#include "cache-item.tmpl.c" > +#undef CLIENT_CURSOR_CACHE > + > +#ifdef DEBUG_CURSORS > +static int _cursor_count = 0; > +#endif > + > +void cursor_channel_client_reset_cursor_cache(RedChannelClient *rcc) > +{ > + red_cursor_cache_reset(RCC_TO_CCC(rcc), CLIENT_CURSOR_CACHE_SIZE); > +} > + > +void cursor_channel_client_on_disconnect(RedChannelClient *rcc) > +{ > + if (!rcc) { > + return; > + } > + cursor_channel_client_reset_cursor_cache(rcc); > +} > + > +void cursor_channel_client_migrate(RedChannelClient *rcc) > +{ > + spice_return_if_fail(rcc); > + > + red_channel_client_pipe_add_type(rcc, > RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE); > + red_channel_client_default_migrate(rcc); > +} > + > +CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, > RedClient *client, RedsStream *stream, > + int mig_target, > + uint32_t *common_caps, int > num_common_caps, > + uint32_t *caps, int num_caps) > +{ > + spice_return_val_if_fail(cursor, NULL); > + spice_return_val_if_fail(client, NULL); > + spice_return_val_if_fail(stream, NULL); > + spice_return_val_if_fail(!num_common_caps || common_caps, NULL); > + spice_return_val_if_fail(!num_caps || caps, NULL); > + > + CursorChannelClient *ccc = > + > (CursorChannelClient*)red_channel_client_create(sizeof(CursorChannelClient), > + RED_CHANNEL(cursor), > + client, stream, > + FALSE, > + num_common_caps, > + common_caps, > + num_caps, > + caps); > + spice_return_val_if_fail(ccc != NULL, NULL); > + COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate = mig_target; > + > + ring_init(&ccc->cursor_cache_lru); > + ccc->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE; > + > + return ccc; > +} > + > +RedCacheItem* cursor_channel_client_cache_find(CursorChannelClient *ccc, > uint64_t id) > +{ > + return red_cursor_cache_find(ccc, id); > +} > + > +int cursor_channel_client_cache_add(CursorChannelClient *ccc, uint64_t id, > size_t size) > +{ > + return red_cursor_cache_add(ccc, id, size); > +} > diff --git a/server/cursor-channel-client.h b/server/cursor-channel-client.h > new file mode 100644 > index 0000000..0760f09 > --- /dev/null > +++ b/server/cursor-channel-client.h > @@ -0,0 +1,50 @@ > +/* -*- Mode: C; c-basic-offset: 4; indent-tabs-mode: nil -*- */ > +/* > + Copyright (C) 2009 Red Hat, Inc. > + > + This library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + This library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with this library; if not, see > <http://www.gnu.org/licenses/>. > +*/ > +#ifndef CURSOR_CHANNEL_CLIENT_H_ > +# define CURSOR_CHANNEL_CLIENT_H_ > + > +#include "cache-item.h" > +#include "red-common.h" > +#include "red-channel.h" > +#include "reds-stream.h" > + > +typedef struct CursorChannel CursorChannel; > +typedef struct CursorChannelClient CursorChannelClient; > + > +#define CURSOR_CHANNEL_CLIENT(Client) ((CursorChannelClient*)(Client)) > + > +CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, > + RedClient *client, > + RedsStream *stream, > + int mig_target, > + uint32_t *common_caps, > + int num_common_caps, > + uint32_t *caps, int > num_caps); > + > +/** > + * Migrate a client channel from a CursorChannel. > + * This is the equivalent of RedChannel client migrate callback. > + * See comment on cursor_channel_new. > + */ > +void cursor_channel_client_migrate(RedChannelClient *rcc); > +void cursor_channel_client_reset_cursor_cache(RedChannelClient *rcc); > +void cursor_channel_client_on_disconnect(RedChannelClient *rcc); > +RedCacheItem* cursor_channel_client_cache_find(CursorChannelClient *ccc, > uint64_t id); > +int cursor_channel_client_cache_add(CursorChannelClient *ccc, uint64_t id, > size_t size); > + > +#endif /* CURSOR_CHANNEL_CLIENT_H_ */ > diff --git a/server/cursor-channel.c b/server/cursor-channel.c > index 83ce61b..591d68f 100644 > --- a/server/cursor-channel.c > +++ b/server/cursor-channel.c > @@ -23,15 +23,7 @@ > #include <common/generated_server_marshallers.h> > > #include "cursor-channel.h" > -#include "red-channel-client-private.h" > -#include "cache-item.h" > - > -#define CLIENT_CURSOR_CACHE_SIZE 256 > - > -#define CURSOR_CACHE_HASH_SHIFT 8 > -#define CURSOR_CACHE_HASH_SIZE (1 << CURSOR_CACHE_HASH_SHIFT) > -#define CURSOR_CACHE_HASH_MASK (CURSOR_CACHE_HASH_SIZE - 1) > -#define CURSOR_CACHE_HASH_KEY(id) ((id) & CURSOR_CACHE_HASH_MASK) > +#include "reds.h" > > typedef struct CursorChannelClient CursorChannelClient; > > @@ -69,23 +61,7 @@ struct CursorChannel { > #endif > }; > > -struct CursorChannelClient { > - RedChannelClient base; > - > - RedCacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE]; > - Ring cursor_cache_lru; > - long cursor_cache_available; > - uint32_t cursor_cache_items; > -}; > - > - > -#define RCC_TO_CCC(rcc) SPICE_CONTAINEROF((rcc), CursorChannelClient, base) > - > -#define CLIENT_CURSOR_CACHE > -#include "cache-item.tmpl.c" > -#undef CLIENT_CURSOR_CACHE > - > -static red_pipe_item_free_t cursor_pipe_item_free; > +static void cursor_pipe_item_free(RedPipeItem *pipe_item); These 2 last lines just change definition style. > > static CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd) > { > @@ -143,8 +119,7 @@ static RedPipeItem *new_cursor_pipe_item(RedChannelClient > *rcc, void *data, int > > red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_CURSOR, > cursor_pipe_item_free); > - item->cursor_item = data; > - item->cursor_item->refs++; > + item->cursor_item = cursor_item_ref(data); > return &item->base; > } > > @@ -175,11 +150,11 @@ static void cursor_fill(CursorChannelClient *ccc, > SpiceCursor *red_cursor, > *red_cursor = cursor_cmd->u.set.shape; > > if (red_cursor->header.unique) { > - if (red_cursor_cache_find(ccc, red_cursor->header.unique)) { > + if (cursor_channel_client_cache_find(ccc, > red_cursor->header.unique)) { > red_cursor->flags |= SPICE_CURSOR_FLAGS_FROM_CACHE; > return; > } > - if (red_cursor_cache_add(ccc, red_cursor->header.unique, 1)) { > + if (cursor_channel_client_cache_add(ccc, red_cursor->header.unique, > 1)) { > red_cursor->flags |= SPICE_CURSOR_FLAGS_CACHE_ME; > } > } > @@ -190,12 +165,6 @@ static void cursor_fill(CursorChannelClient *ccc, > SpiceCursor *red_cursor, > } > } > > - > -static void red_reset_cursor_cache(RedChannelClient *rcc) > -{ > - red_cursor_cache_reset(RCC_TO_CCC(rcc), CLIENT_CURSOR_CACHE_SIZE); > -} > - > void cursor_channel_disconnect(CursorChannel *cursor_channel) > { > RedChannel *channel = (RedChannel *)cursor_channel; > @@ -203,11 +172,10 @@ void cursor_channel_disconnect(CursorChannel > *cursor_channel) > if (!channel || !red_channel_is_connected(channel)) { > return; > } > - red_channel_apply_clients(channel, red_reset_cursor_cache); > + red_channel_apply_clients(channel, > cursor_channel_client_reset_cursor_cache); > red_channel_disconnect(channel); > } > > - > static void cursor_pipe_item_free(RedPipeItem *base) > { > spice_return_if_fail(base); > @@ -220,14 +188,6 @@ static void cursor_pipe_item_free(RedPipeItem *base) > free(pipe_item); > } > > -static void cursor_channel_client_on_disconnect(RedChannelClient *rcc) > -{ > - if (!rcc) { > - return; > - } > - red_reset_cursor_cache(rcc); > -} > - > static void red_marshall_cursor_init(CursorChannelClient *ccc, > SpiceMarshaller *base_marshaller, > RedPipeItem *pipe_item) > { > @@ -237,7 +197,7 @@ static void red_marshall_cursor_init(CursorChannelClient > *ccc, SpiceMarshaller * > AddBufInfo info; > > spice_assert(rcc); > - cursor_channel = SPICE_CONTAINEROF(rcc->channel, CursorChannel, > common.base); > + cursor_channel = (CursorChannel*)red_channel_client_get_channel(rcc); > > red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INIT, NULL); > msg.visible = cursor_channel->cursor_visible; > @@ -255,7 +215,7 @@ static void cursor_marshall(CursorChannelClient *ccc, > RedCursorPipeItem *cursor_pipe_item) > { > RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc); > - CursorChannel *cursor_channel = SPICE_CONTAINEROF(rcc->channel, > CursorChannel, common.base); > + CursorChannel *cursor_channel = > SPICE_CONTAINEROF(red_channel_client_get_channel(rcc), CursorChannel, > common.base); Here SPICE_CONTAINEROF is still used above there is a simple cast. I prefer this one but in any case I would do a coherent change. > CursorItem *item = cursor_pipe_item->cursor_item; > RedPipeItem *pipe_item = &cursor_pipe_item->base; > RedCursorCmd *cmd; > @@ -319,7 +279,7 @@ static inline void red_marshall_inval(RedChannelClient > *rcc, > static void cursor_channel_send_item(RedChannelClient *rcc, RedPipeItem > *pipe_item) > { > SpiceMarshaller *m = red_channel_client_get_marshaller(rcc); > - CursorChannelClient *ccc = RCC_TO_CCC(rcc); > + CursorChannelClient *ccc = (CursorChannelClient *)rcc; > I think we defined a macro for this > switch (pipe_item->type) { > case RED_PIPE_ITEM_TYPE_CURSOR: > @@ -332,11 +292,11 @@ static void cursor_channel_send_item(RedChannelClient > *rcc, RedPipeItem *pipe_it > red_marshall_verb(rcc, SPICE_UPCAST(RedVerbItem, pipe_item)); > break; > case RED_PIPE_ITEM_TYPE_CURSOR_INIT: > - red_reset_cursor_cache(rcc); > + cursor_channel_client_reset_cursor_cache(rcc); > red_marshall_cursor_init(ccc, m, pipe_item); > break; > case RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE: > - red_reset_cursor_cache(rcc); > + cursor_channel_client_reset_cursor_cache(rcc); > red_channel_client_init_send_data(rcc, SPICE_MSG_CURSOR_INVAL_ALL, > NULL); > break; > default: > @@ -367,45 +327,6 @@ CursorChannel* cursor_channel_new(RedWorker *worker) > return cursor_channel; > } > > -void cursor_channel_client_migrate(RedChannelClient *rcc) > -{ > - spice_return_if_fail(rcc); > - > - red_channel_client_pipe_add_type(rcc, > RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE); > - red_channel_client_default_migrate(rcc); > -} > - > -static CursorChannelClient* cursor_channel_client_new(CursorChannel *cursor, > - RedClient *client, > - RedsStream *stream, > - int mig_target, > - uint32_t *common_caps, > int num_common_caps, > - uint32_t *caps, int > num_caps) > -{ > - spice_return_val_if_fail(cursor, NULL); > - spice_return_val_if_fail(client, NULL); > - spice_return_val_if_fail(stream, NULL); > - spice_return_val_if_fail(!num_common_caps || common_caps, NULL); > - spice_return_val_if_fail(!num_caps || caps, NULL); > - > - CursorChannelClient *ccc = > - > (CursorChannelClient*)red_channel_client_create(sizeof(CursorChannelClient), > - > &cursor->common.base, > - client, stream, > - FALSE, > - num_common_caps, > - common_caps, > - num_caps, > - caps); > - spice_return_val_if_fail(ccc != NULL, NULL); > - cursor->common.during_target_migrate = mig_target; > - > - ring_init(&ccc->cursor_cache_lru); > - ccc->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE; > - > - return ccc; > -} > - > void cursor_channel_process_cmd(CursorChannel *cursor, RedCursorCmd > *cursor_cmd) > { > CursorItem *cursor_item; > diff --git a/server/cursor-channel.h b/server/cursor-channel.h > index 6aeac16..0cff16c 100644 > --- a/server/cursor-channel.h > +++ b/server/cursor-channel.h > @@ -18,10 +18,8 @@ > #ifndef CURSOR_CHANNEL_H_ > # define CURSOR_CHANNEL_H_ > > -#include "spice.h" > -#include "reds.h" > +#include "cursor-channel-client.h" cursor-channel-client.h should not be included here. > #include "red-worker.h" > -#include "red-parse-qxl.h" > > /** > * This type it's a RedChannel class which implement cursor (mouse) > @@ -29,6 +27,7 @@ > * A pointer to CursorChannel can be converted to a RedChannel. > */ > typedef struct CursorChannel CursorChannel; > +typedef struct CursorItem CursorItem; > No reason to expose this define. > /** > * Create CursorChannel. > @@ -64,11 +63,4 @@ void cursor_channel_connect > (CursorChannel *cursor, RedClien > uint32_t *common_caps, int > num_common_caps, > uint32_t *caps, int > num_caps); > > -/** > - * Migrate a client channel from a CursorChannel. > - * This is the equivalent of RedChannel client migrate callback. > - * See comment on cursor_channel_new. > - */ > -void cursor_channel_client_migrate(RedChannelClient > *client); > - Better to keep here. > #endif /* CURSOR_CHANNEL_H_ */ > diff --git a/server/red-worker.c b/server/red-worker.c > index 590412b..eab8c64 100644 > --- a/server/red-worker.c > +++ b/server/red-worker.c > @@ -50,6 +50,7 @@ > #include "spice.h" > #include "red-worker.h" > #include "cursor-channel.h" > +#include "cursor-channel-client.h" > #include "tree.h" > > #define CMD_RING_POLL_TIMEOUT 10 //milli This hunk should be removed. Frediano _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel