[Spice-devel] [PATCH 02/11] Move CursorChannelClient to separate file
Frediano Ziglio
fziglio at redhat.com
Thu Aug 11 09:06:41 UTC 2016
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
More information about the Spice-devel
mailing list