[Spice-devel] [PATCH 09/10] Add DisplayChannelClientPrivate and CursorChannelPrivate structs

Frediano Ziglio fziglio at redhat.com
Thu Sep 1 12:29:13 UTC 2016


> 
> Hi,
> 
> On Wed, Aug 31, 2016 at 11:54:45AM -0500, Jonathon Jongsma wrote:
> > These need to be introduced at the same time since cache-item.tmpl.c
> > assumes that both of these classes will have a cache in the same place:
> > either within the channel client struct itself or (now) within a priv
> > struct owned by the channel client.
> > 
> > This encapsulates private data and prepares for porting to GObject.
> > ---
> >  server/cache-item.tmpl.c       |  38 +++++-----
> >  server/cursor-channel-client.c |  14 +++-
> >  server/dcc-private.h           |  10 ++-
> >  server/dcc-send.c              | 114 ++++++++++++++--------------
> >  server/dcc.c                   | 166
> >  +++++++++++++++++++++--------------------
> >  5 files changed, 181 insertions(+), 161 deletions(-)
> > 
> > diff --git a/server/cache-item.tmpl.c b/server/cache-item.tmpl.c
> > index d1310a5..ce38a2a 100644
> > --- a/server/cache-item.tmpl.c
> > +++ b/server/cache-item.tmpl.c
> > @@ -46,12 +46,12 @@
> >  
> >  static RedCacheItem *FUNC_NAME(find)(CHANNELCLIENT *channel_client,
> >  uint64_t id)
> >  {
> > -    RedCacheItem *item = channel_client->CACHE_NAME[CACHE_HASH_KEY(id)];
> > +    RedCacheItem *item =
> > channel_client->priv->CACHE_NAME[CACHE_HASH_KEY(id)];
> >  
> >      while (item) {
> >          if (item->id == id) {
> >              ring_remove(&item->u.cache_data.lru_link);
> > -            ring_add(&channel_client->VAR_NAME(lru),
> > &item->u.cache_data.lru_link);
> > +            ring_add(&channel_client->priv->VAR_NAME(lru),
> > &item->u.cache_data.lru_link);
> >              break;
> >          }
> >          item = item->u.cache_data.next;
> > @@ -64,7 +64,7 @@ static void FUNC_NAME(remove)(CHANNELCLIENT
> > *channel_client, RedCacheItem *item)
> >      RedCacheItem **now;
> >      spice_assert(item);
> >  
> > -    now = &channel_client->CACHE_NAME[CACHE_HASH_KEY(item->id)];
> > +    now = &channel_client->priv->CACHE_NAME[CACHE_HASH_KEY(item->id)];
> >      for (;;) {
> >          spice_assert(*now);
> >          if (*now == item) {
> > @@ -74,8 +74,8 @@ static void FUNC_NAME(remove)(CHANNELCLIENT
> > *channel_client, RedCacheItem *item)
> >          now = &(*now)->u.cache_data.next;
> >      }
> >      ring_remove(&item->u.cache_data.lru_link);
> > -    channel_client->VAR_NAME(items)--;
> > -    channel_client->VAR_NAME(available) += item->u.cache_data.size;
> > +    channel_client->priv->VAR_NAME(items)--;
> > +    channel_client->priv->VAR_NAME(available) += item->u.cache_data.size;
> >  
> >      red_pipe_item_init(&item->u.pipe_data, RED_PIPE_ITEM_TYPE_INVAL_ONE);
> >      red_channel_client_pipe_add_tail_and_push(&channel_client->base,
> >      &item->u.pipe_data); // for now
> > @@ -88,22 +88,22 @@ static int FUNC_NAME(add)(CHANNELCLIENT
> > *channel_client, uint64_t id, size_t siz
> >  
> >      item = spice_new(RedCacheItem, 1);
> >  
> > -    channel_client->VAR_NAME(available) -= size;
> > +    channel_client->priv->VAR_NAME(available) -= size;
> >      verify(SPICE_OFFSETOF(RedCacheItem, u.cache_data.lru_link) == 0);
> > -    while (channel_client->VAR_NAME(available) < 0) {
> > -        RedCacheItem *tail = (RedCacheItem
> > *)ring_get_tail(&channel_client->VAR_NAME(lru));
> > +    while (channel_client->priv->VAR_NAME(available) < 0) {
> > +        RedCacheItem *tail = (RedCacheItem
> > *)ring_get_tail(&channel_client->priv->VAR_NAME(lru));
> >          if (!tail) {
> > -            channel_client->VAR_NAME(available) += size;
> > +            channel_client->priv->VAR_NAME(available) += size;
> >              free(item);
> >              return FALSE;
> >          }
> >          FUNC_NAME(remove)(channel_client, tail);
> >      }
> > -    ++channel_client->VAR_NAME(items);
> > -    item->u.cache_data.next = channel_client->CACHE_NAME[(key =
> > CACHE_HASH_KEY(id))];
> > -    channel_client->CACHE_NAME[key] = item;
> > +    ++channel_client->priv->VAR_NAME(items);
> > +    item->u.cache_data.next = channel_client->priv->CACHE_NAME[(key =
> > CACHE_HASH_KEY(id))];
> > +    channel_client->priv->CACHE_NAME[key] = item;
> >      ring_item_init(&item->u.cache_data.lru_link);
> > -    ring_add(&channel_client->VAR_NAME(lru),
> > &item->u.cache_data.lru_link);
> > +    ring_add(&channel_client->priv->VAR_NAME(lru),
> > &item->u.cache_data.lru_link);
> >      item->id = id;
> >      item->u.cache_data.size = size;
> >      return TRUE;
> > @@ -114,15 +114,15 @@ static void FUNC_NAME(reset)(CHANNELCLIENT
> > *channel_client, long size)
> >      int i;
> >  
> >      for (i = 0; i < CACHE_HASH_SIZE; i++) {
> > -        while (channel_client->CACHE_NAME[i]) {
> > -            RedCacheItem *item = channel_client->CACHE_NAME[i];
> > -            channel_client->CACHE_NAME[i] = item->u.cache_data.next;
> > +        while (channel_client->priv->CACHE_NAME[i]) {
> > +            RedCacheItem *item = channel_client->priv->CACHE_NAME[i];
> > +            channel_client->priv->CACHE_NAME[i] = item->u.cache_data.next;
> >              free(item);
> >          }
> >      }
> > -    ring_init(&channel_client->VAR_NAME(lru));
> > -    channel_client->VAR_NAME(available) = size;
> > -    channel_client->VAR_NAME(items) = 0;
> > +    ring_init(&channel_client->priv->VAR_NAME(lru));
> > +    channel_client->priv->VAR_NAME(available) = size;
> > +    channel_client->priv->VAR_NAME(items) = 0;
> >  }
> >  
> >  
> > diff --git a/server/cursor-channel-client.c
> > b/server/cursor-channel-client.c
> > index 8c26d43..dceb2d0 100644
> > --- a/server/cursor-channel-client.c
> > +++ b/server/cursor-channel-client.c
> > @@ -39,9 +39,16 @@ enum {
> >      RED_PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE,
> >  };
> >  
> > -struct CursorChannelClient {
> > +typedef struct CursorChannelClientPrivate CursorChannelClientPrivate;
> > +struct CursorChannelClient
> > +{
> >      RedChannelClient base;
> >  
> > +    CursorChannelClientPrivate *priv;
> > +};
> > +
> > +struct CursorChannelClientPrivate
> > +{
> >      RedCacheItem *cursor_cache[CURSOR_CACHE_HASH_SIZE];
> >      Ring cursor_cache_lru;
> >      long cursor_cache_available;
> > @@ -99,10 +106,11 @@ CursorChannelClient*
> > cursor_channel_client_new(CursorChannel *cursor, RedClient
> >                                                          num_caps,
> >                                                          caps);
> >      spice_return_val_if_fail(ccc != NULL, NULL);
> > +    ccc->priv = g_new0(CursorChannelClientPrivate, 1);
> >      COMMON_GRAPHICS_CHANNEL(cursor)->during_target_migrate = mig_target;
> >  
> > -    ring_init(&ccc->cursor_cache_lru);
> > -    ccc->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE;
> > +    ring_init(&ccc->priv->cursor_cache_lru);
> > +    ccc->priv->cursor_cache_available = CLIENT_CURSOR_CACHE_SIZE;
> >  
> >      return ccc;
> >  }
> > diff --git a/server/dcc-private.h b/server/dcc-private.h
> > index d5aad3f..1c8fe9a 100644
> > --- a/server/dcc-private.h
> > +++ b/server/dcc-private.h
> > @@ -24,9 +24,17 @@
> >  #include "stream.h"
> >  #include "red-channel-client.h"
> >  
> > -struct DisplayChannelClient {
> > +typedef struct DisplayChannelClientPrivate DisplayChannelClientPrivate;
> > +struct DisplayChannelClient
> > +{
> >      RedChannelClient base;
> >      int is_low_bandwidth;
> 
> Shouldn't it be on priv too?
> 
>   toso
> 

Yes, but if you do it will crash!
At least now. The problem is the initialization of the priv pointer done
too late.

...

Frediano


More information about the Spice-devel mailing list