Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx> On Fri, 2016-05-20 at 14:01 +0100, Frediano Ziglio wrote: > Scan remaining code searching for problems with structure > layout assumptions in the code. > Where code required some restructuring put some verify checks > to make sure code won't compile if these assumptions are not > in place anymore. > > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx> > --- > server/cache-item.tmpl.c | 1 + > server/cursor-channel.c | 4 ++-- > server/dcc-encoders.c | 2 +- > server/dcc-send.c | 14 ++++++-------- > server/dcc.c | 5 +++-- > server/dispatcher.c | 3 +-- > server/display-channel.c | 26 ++++++++++++++------------ > server/image-cache.c | 7 +++++-- > server/main-channel.c | 5 ++--- > server/pixmap-cache.c | 3 ++- > server/red-replay-qxl.c | 2 +- > server/smartcard.c | 4 ++-- > server/tree.c | 12 +++++++----- > server/tree.h | 8 ++++---- > 14 files changed, 51 insertions(+), 45 deletions(-) > > diff --git a/server/cache-item.tmpl.c b/server/cache-item.tmpl.c > index 0617bea..0f22b35 100644 > --- a/server/cache-item.tmpl.c > +++ b/server/cache-item.tmpl.c > @@ -93,6 +93,7 @@ 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; > + 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)); > if (!tail) { > diff --git a/server/cursor-channel.c b/server/cursor-channel.c > index c257654..23a8cb8 100644 > --- a/server/cursor-channel.c > +++ b/server/cursor-channel.c > @@ -321,10 +321,10 @@ static void cursor_channel_send_item(RedChannelClient > *rcc, RedPipeItem *pipe_it > cursor_marshall(rcc, m, SPICE_CONTAINEROF(pipe_item, > RedCursorPipeItem, base)); > break; > case RED_PIPE_ITEM_TYPE_INVAL_ONE: > - red_marshall_inval(rcc, m, (RedCacheItem *)pipe_item); > + red_marshall_inval(rcc, m, SPICE_CONTAINEROF(pipe_item, RedCacheItem, > u.pipe_data)); > break; > case RED_PIPE_ITEM_TYPE_VERB: > - red_marshall_verb(rcc, (RedVerbItem*)pipe_item); > + red_marshall_verb(rcc, SPICE_CONTAINEROF(pipe_item, RedVerbItem, > base)); > break; > case RED_PIPE_ITEM_TYPE_CURSOR_INIT: > red_reset_cursor_cache(rcc); > diff --git a/server/dcc-encoders.c b/server/dcc-encoders.c > index f1dd1bb..65f5a17 100644 > --- a/server/dcc-encoders.c > +++ b/server/dcc-encoders.c > @@ -624,7 +624,7 @@ static GlzSharedDictionary *find_glz_dictionary(RedClient > *client, uint8_t dict_ > > now = &glz_dictionary_list; > while ((now = ring_next(&glz_dictionary_list, now))) { > - GlzSharedDictionary *dict = (GlzSharedDictionary *)now; > + GlzSharedDictionary *dict = SPICE_CONTAINEROF(now, > GlzSharedDictionary, base); > if ((dict->client == client) && (dict->id == dict_id)) { > ret = dict; > break; > diff --git a/server/dcc-send.c b/server/dcc-send.c > index 44b8448..91adbd7 100644 > --- a/server/dcc-send.c > +++ b/server/dcc-send.c > @@ -2381,34 +2381,32 @@ void dcc_send_item(RedChannelClient *rcc, RedPipeItem > *pipe_item) > break; > } > case RED_PIPE_ITEM_TYPE_INVAL_ONE: > - marshall_inval_palette(rcc, m, (RedCacheItem *)pipe_item); > + marshall_inval_palette(rcc, m, SPICE_CONTAINEROF(pipe_item, > RedCacheItem, u.pipe_data)); > break; > case RED_PIPE_ITEM_TYPE_STREAM_CREATE: { > StreamCreateDestroyItem *item = SPICE_CONTAINEROF(pipe_item, > StreamCreateDestroyItem, base); > marshall_stream_start(rcc, m, item->agent); > break; > } > - case RED_PIPE_ITEM_TYPE_STREAM_CLIP: { > - RedStreamClipItem* clip_item = (RedStreamClipItem *)pipe_item; > - marshall_stream_clip(rcc, m, clip_item); > + case RED_PIPE_ITEM_TYPE_STREAM_CLIP: > + marshall_stream_clip(rcc, m, SPICE_CONTAINEROF(pipe_item, > RedStreamClipItem, base)); > break; > - } > case RED_PIPE_ITEM_TYPE_STREAM_DESTROY: { > StreamCreateDestroyItem *item = SPICE_CONTAINEROF(pipe_item, > StreamCreateDestroyItem, base); > marshall_stream_end(rcc, m, item->agent); > break; > } > case RED_PIPE_ITEM_TYPE_UPGRADE: > - marshall_upgrade(rcc, m, (RedUpgradeItem *)pipe_item); > + marshall_upgrade(rcc, m, SPICE_CONTAINEROF(pipe_item, RedUpgradeItem, > base)); > break; > case RED_PIPE_ITEM_TYPE_VERB: > - red_marshall_verb(rcc, (RedVerbItem*)pipe_item); > + red_marshall_verb(rcc, SPICE_CONTAINEROF(pipe_item, RedVerbItem, > base)); > break; > case RED_PIPE_ITEM_TYPE_MIGRATE_DATA: > display_channel_marshall_migrate_data(rcc, m); > break; > case RED_PIPE_ITEM_TYPE_IMAGE: > - red_marshall_image(rcc, m, (RedImageItem *)pipe_item); > + red_marshall_image(rcc, m, SPICE_CONTAINEROF(pipe_item, RedImageItem, > base)); > break; > case RED_PIPE_ITEM_TYPE_PIXMAP_SYNC: > display_channel_marshall_pixmap_sync(rcc, m); > diff --git a/server/dcc.c b/server/dcc.c > index df5123b..66ac710 100644 > --- a/server/dcc.c > +++ b/server/dcc.c > @@ -90,7 +90,7 @@ int > dcc_clear_surface_drawables_from_pipe(DisplayChannelClient *dcc, int surface > dpi = SPICE_CONTAINEROF(item, RedDrawablePipeItem, > dpi_pipe_item); > drawable = dpi->drawable; > } else if (item->type == RED_PIPE_ITEM_TYPE_UPGRADE) { > - drawable = ((RedUpgradeItem *)item)->drawable; > + drawable = SPICE_CONTAINEROF(item, RedUpgradeItem, base) > ->drawable; > } else { > continue; > } > @@ -533,7 +533,7 @@ static RedMonitorsConfigItem > *red_monitors_config_item_new(RedChannel* channel, > { > RedMonitorsConfigItem *mci; > > - mci = (RedMonitorsConfigItem *)spice_malloc(sizeof(*mci)); > + mci = spice_new(RedMonitorsConfigItem, 1); > mci->monitors_config = monitors_config; > > red_pipe_item_init_full(&mci->pipe_item, > RED_PIPE_ITEM_TYPE_MONITORS_CONFIG, > @@ -1326,6 +1326,7 @@ int dcc_pixmap_cache_unlocked_add(DisplayChannelClient > *dcc, uint64_t id, > NewCacheItem *tail; > NewCacheItem **now; > > + verify(SPICE_OFFSETOF(NewCacheItem, lru_link) == 0); > if (!(tail = (NewCacheItem *)ring_get_tail(&cache->lru)) || > tail->sync[dcc->id] == > serial) { > cache->available += size; > diff --git a/server/dispatcher.c b/server/dispatcher.c > index b9e23f6..8c55881 100644 > --- a/server/dispatcher.c > +++ b/server/dispatcher.c > @@ -299,8 +299,7 @@ static int dispatcher_handle_single_read(Dispatcher > *dispatcher) > /* TODO: close socketpair? */ > } > } else if (msg->ack == DISPATCHER_ASYNC && dispatcher->priv > ->handle_async_done) { > - dispatcher->priv->handle_async_done(dispatcher->priv->opaque, type, > - (void *)payload); > + dispatcher->priv->handle_async_done(dispatcher->priv->opaque, type, > payload); > } > return 1; > } > diff --git a/server/display-channel.c b/server/display-channel.c > index 9f97911..96a6f2f 100644 > --- a/server/display-channel.c > +++ b/server/display-channel.c > @@ -390,7 +390,7 @@ static void current_remove(DisplayChannel *display, > TreeItem *item) > drawable_remove_from_pipes(drawable); > current_remove_drawable(display, drawable); > } else { > - Container *container = (Container *)now; > + Container *container = CONTAINER(now); > > spice_assert(now->type == TREE_ITEM_TYPE_CONTAINER); > > @@ -408,7 +408,7 @@ static void current_remove(DisplayChannel *display, > TreeItem *item) > if ((ring_item = ring_next(&container->items, ring_item))) { > now = SPICE_CONTAINEROF(ring_item, TreeItem, siblings_link); > } else { > - now = (TreeItem *)container; > + now = &container->base; > } > } > } > @@ -433,7 +433,7 @@ static int current_add_equal(DisplayChannel *display, > DrawItem *item, TreeItem * > if (other->type != TREE_ITEM_TYPE_DRAWABLE) { > return FALSE; > } > - other_draw_item = (DrawItem *)other; > + other_draw_item = DRAW_ITEM(other); > > if (item->shadow || other_draw_item->shadow || item->effect != > other_draw_item->effect) { > return FALSE; > @@ -530,7 +530,7 @@ static void __exclude_region(DisplayChannel *display, Ring > *ring, TreeItem *item > region_and(&and_rgn, &item->rgn); > if (!region_is_empty(&and_rgn)) { > if (IS_DRAW_ITEM(item)) { > - DrawItem *draw = (DrawItem *)item; > + DrawItem *draw = DRAW_ITEM(item); > > if (draw->effect == QXL_EFFECT_OPAQUE) { > region_exclude(rgn, &and_rgn); > @@ -551,8 +551,8 @@ static void __exclude_region(DisplayChannel *display, Ring > *ring, TreeItem *item > region_exclude(&shadow->on_hold, &and_rgn); > region_or(rgn, &and_rgn); > // in flat representation of current, shadow is always > his owner next > - if (!tree_item_contained_by((TreeItem*)shadow, > *top_ring)) { > - *top_ring = > tree_item_container_items((TreeItem*)shadow, ring); > + if (!tree_item_contained_by(&shadow->base, *top_ring)) { > + *top_ring = tree_item_container_items(&shadow->base, > ring); > } > } > } else { > @@ -571,8 +571,8 @@ static void __exclude_region(DisplayChannel *display, Ring > *ring, TreeItem *item > region_exclude(rgn, &and_rgn); > if ((shadow = tree_item_find_shadow(item))) { > region_or(rgn, &shadow->on_hold); > - if (!tree_item_contained_by((TreeItem*)shadow, > *top_ring)) { > - *top_ring = > tree_item_container_items((TreeItem*)shadow, ring); > + if (!tree_item_contained_by(&shadow->base, *top_ring)) { > + *top_ring = tree_item_container_items(&shadow->base, > ring); > } > } > } > @@ -580,7 +580,7 @@ static void __exclude_region(DisplayChannel *display, Ring > *ring, TreeItem *item > Shadow *shadow; > > spice_assert(item->type == TREE_ITEM_TYPE_SHADOW); > - shadow = (Shadow *)item; > + shadow = SHADOW(item); > region_exclude(rgn, &and_rgn); > region_or(&shadow->on_hold, &and_rgn); > } > @@ -615,13 +615,14 @@ static void exclude_region(DisplayChannel *display, Ring > *ring, RingItem *ring_i > ring_item = now->siblings_link.prev; > current_remove(display, now); > if (last && *last == now) { > + verify(SPICE_OFFSETOF(TreeItem, siblings_link) == 0); > *last = (TreeItem *)ring_next(ring, ring_item); > } > } else if (now->type == TREE_ITEM_TYPE_CONTAINER) { > - Container *container = (Container *)now; > + Container *container = CONTAINER(now); > if ((ring_item = ring_get_head(&container->items))) { > ring = &container->items; > - spice_assert(((TreeItem *)ring_item)->container); > + spice_assert(SPICE_CONTAINEROF(ring_item, TreeItem, > siblings_link)->container); > continue; > } > ring_item = &now->siblings_link; > @@ -633,6 +634,7 @@ static void exclude_region(DisplayChannel *display, Ring > *ring, RingItem *ring_i > } > } > > + verify(SPICE_OFFSETOF(TreeItem, siblings_link) == 0); > while ((last && *last == (TreeItem *)ring_item) || > !(ring_item = ring_next(ring, ring_item))) { > if (ring == top_ring) { > @@ -755,7 +757,7 @@ static int current_add(DisplayChannel *display, Ring > *ring, Drawable *drawable) > exclude_base = NULL; > } > if (sibling->type == TREE_ITEM_TYPE_CONTAINER) { > - container = (Container *)sibling; > + container = CONTAINER(sibling); > ring = &container->items; > item->base.container = container; > now = ring_next(ring, ring); > diff --git a/server/image-cache.c b/server/image-cache.c > index 4237034..8fb090f 100644 > --- a/server/image-cache.c > +++ b/server/image-cache.c > @@ -74,11 +74,12 @@ static void image_cache_remove(ImageCache *cache, > ImageCacheItem *item) > > static void image_cache_put(SpiceImageCache *spice_cache, uint64_t id, > pixman_image_t *image) > { > - ImageCache *cache = (ImageCache *)spice_cache; > + ImageCache *cache = SPICE_CONTAINEROF(spice_cache, ImageCache, base); > ImageCacheItem *item; > > #ifndef IMAGE_CACHE_AGE > if (cache->num_items == IMAGE_CACHE_MAX_ITEMS) { > + verify(SPICE_OFFSETOF(ImageCacheItem, lru_link) == 0); > ImageCacheItem *tail = (ImageCacheItem *)ring_get_tail(&cache->lru); > spice_assert(tail); > image_cache_remove(cache, tail); > @@ -103,7 +104,7 @@ static void image_cache_put(SpiceImageCache *spice_cache, > uint64_t id, pixman_im > > static pixman_image_t *image_cache_get(SpiceImageCache *spice_cache, uint64_t > id) > { > - ImageCache *cache = (ImageCache *)spice_cache; > + ImageCache *cache = SPICE_CONTAINEROF(spice_cache, ImageCache,base); > > ImageCacheItem *item = image_cache_find(cache, id); > if (!item) { > @@ -133,6 +134,7 @@ void image_cache_reset(ImageCache *cache) > { > ImageCacheItem *item; > > + verify(SPICE_OFFSETOF(ImageCacheItem, lru_link) == 0); > while ((item = (ImageCacheItem *)ring_get_head(&cache->lru))) { > image_cache_remove(cache, item); > } > @@ -145,6 +147,7 @@ void image_cache_reset(ImageCache *cache) > > void image_cache_aging(ImageCache *cache) > { > + verify(SPICE_OFFSETOF(ImageCacheItem, lru_link) == 0); > #ifdef IMAGE_CACHE_AGE > ImageCacheItem *item; > > diff --git a/server/main-channel.c b/server/main-channel.c > index e89af97..ca2d6d9 100644 > --- a/server/main-channel.c > +++ b/server/main-channel.c > @@ -314,10 +314,9 @@ static void main_notify_item_free(RedPipeItem *base) > free(data); > } > > -static RedPipeItem *main_notify_item_new(RedChannelClient *rcc, void *data, > int num) > +static RedPipeItem *main_notify_item_new(RedChannelClient *rcc, const char > *msg, int num) > { > RedNotifyPipeItem *item = spice_malloc(sizeof(RedNotifyPipeItem)); > - const char *msg = data; > > red_pipe_item_init_full(&item->base, RED_PIPE_ITEM_TYPE_MAIN_NOTIFY, > main_notify_item_free); > @@ -585,7 +584,7 @@ void main_channel_client_push_uuid(MainChannelClient *mcc, > const uint8_t uuid[16 > > void main_channel_client_push_notify(MainChannelClient *mcc, const char *msg) > { > - RedPipeItem *item = main_notify_item_new(&mcc->base, (void *)msg, 1); > + RedPipeItem *item = main_notify_item_new(&mcc->base, msg, 1); > red_channel_client_pipe_add_push(&mcc->base, item); > } > > diff --git a/server/pixmap-cache.c b/server/pixmap-cache.c > index a485268..6b818d8 100644 > --- a/server/pixmap-cache.c > +++ b/server/pixmap-cache.c > @@ -46,6 +46,7 @@ void pixmap_cache_clear(PixmapCache *cache) > cache->freezed = FALSE; > } > > + verify(SPICE_OFFSETOF(NewCacheItem, lru_link) == 0); > while ((item = (NewCacheItem *)ring_get_head(&cache->lru))) { > ring_remove(&item->lru_link); > free(item); > @@ -113,7 +114,7 @@ PixmapCache *pixmap_cache_get(RedClient *client, uint8_t > id, int64_t size) > > now = &pixmap_cache_list; > while ((now = ring_next(&pixmap_cache_list, now))) { > - PixmapCache *cache = (PixmapCache *)now; > + PixmapCache *cache = SPICE_CONTAINEROF(now, PixmapCache, base); > if ((cache->client == client) && (cache->id == id)) { > ret = cache; > ret->refs++; > diff --git a/server/red-replay-qxl.c b/server/red-replay-qxl.c > index 60e4183..17019f8 100644 > --- a/server/red-replay-qxl.c > +++ b/server/red-replay-qxl.c > @@ -369,7 +369,7 @@ static QXLImage *red_replay_image(SpiceReplay *replay, > uint32_t flags) > return NULL; > } > > - qxl = (QXLImage*)spice_malloc0(sizeof(QXLImage)); > + qxl = spice_new0(QXLImage, 1); > replay_fscanf(replay, "descriptor.id %"PRIu64"\n", &qxl->descriptor.id); > replay_fscanf(replay, "descriptor.type %d\n", &temp); qxl > ->descriptor.type = temp; > replay_fscanf(replay, "descriptor.flags %d\n", &temp); qxl > ->descriptor.flags = temp; > diff --git a/server/smartcard.c b/server/smartcard.c > index 9280038..f68ce48 100644 > --- a/server/smartcard.c > +++ b/server/smartcard.c > @@ -440,7 +440,7 @@ static void smartcard_channel_send_data(RedChannelClient > *rcc, SpiceMarshaller * > static void smartcard_channel_send_error( > RedChannelClient *rcc, SpiceMarshaller *m, RedPipeItem *item) > { > - RedErrorItem* error_item = (RedErrorItem*)item; > + RedErrorItem* error_item = SPICE_CONTAINEROF(item, RedErrorItem, base); > > smartcard_channel_send_data(rcc, m, item, &error_item->vheader); > } > @@ -448,7 +448,7 @@ static void smartcard_channel_send_error( > static void smartcard_channel_send_msg(RedChannelClient *rcc, > SpiceMarshaller *m, RedPipeItem *item) > { > - RedMsgItem* msg_item = (RedMsgItem*)item; > + RedMsgItem* msg_item = SPICE_CONTAINEROF(item, RedMsgItem, base); > > smartcard_channel_send_data(rcc, m, item, msg_item->vheader); > } > diff --git a/server/tree.c b/server/tree.c > index 9e5a281..7da8d7d 100644 > --- a/server/tree.c > +++ b/server/tree.c > @@ -153,7 +153,7 @@ static void dump_item(TreeItem *item, void *data) > } > case TREE_ITEM_TYPE_CONTAINER: > di->level++; > - di->container = (Container *)item; > + di->container = CONTAINER(item); > break; > case TREE_ITEM_TYPE_SHADOW: > break; > @@ -168,7 +168,7 @@ static void tree_foreach(TreeItem *item, void > (*f)(TreeItem *, void *), void * d > f(item, data); > > if (item->type == TREE_ITEM_TYPE_CONTAINER) { > - Container *container = (Container*)item; > + Container *container = CONTAINER(item); > RingItem *it; > > RING_FOREACH(it, &container->items) { > @@ -240,6 +240,7 @@ void container_cleanup(Container *container) > while (container && container->items.next == container->items.prev) { > Container *next = container->base.container; > if (container->items.next != &container->items) { > + verify(SPICE_OFFSETOF(TreeItem, siblings_link) == 0); > TreeItem *item = (TreeItem *)ring_get_head(&container->items); > spice_assert(item); > ring_remove(&item->siblings_link); > @@ -255,7 +256,8 @@ void container_cleanup(Container *container) > Shadow* tree_item_find_shadow(TreeItem *item) > { > while (item->type == TREE_ITEM_TYPE_CONTAINER) { > - if (!(item = (TreeItem *)ring_get_tail(&((Container *)item)->items))) > { > + verify(SPICE_OFFSETOF(TreeItem, siblings_link) == 0); > + if (!(item = (TreeItem *)ring_get_tail(&CONTAINER(item)->items))) { > return NULL; > } > } > @@ -264,7 +266,7 @@ Shadow* tree_item_find_shadow(TreeItem *item) > return NULL; > } > > - return ((DrawItem *)item)->shadow; > + return DRAW_ITEM(item)->shadow; > } > > Ring *tree_item_container_items(TreeItem *item, Ring *ring) > @@ -280,7 +282,7 @@ int tree_item_contained_by(TreeItem *item, Ring *ring) > if (now == ring) { > return TRUE; > } > - } while ((item = (TreeItem *)item->container)); > + } while ((item = &item->container->base)); > > return FALSE; > } > diff --git a/server/tree.h b/server/tree.h > index 5260ce8..e8da58d 100644 > --- a/server/tree.h > +++ b/server/tree.h > @@ -54,7 +54,7 @@ struct Shadow { > }; > > #define IS_SHADOW(item) ((item)->type == TREE_ITEM_TYPE_SHADOW) > -#define SHADOW(item) ((Shadow*)(item)) > +#define SHADOW(item) SPICE_CONTAINEROF(item, Shadow, base) > > struct Container { > TreeItem base; > @@ -62,7 +62,7 @@ struct Container { > }; > > #define IS_CONTAINER(item) ((item)->type == TREE_ITEM_TYPE_CONTAINER) > -#define CONTAINER(item) ((Container*)(item)) > +#define CONTAINER(item) SPICE_CONTAINEROF(item, Container, base) > > struct DrawItem { > TreeItem base; > @@ -72,12 +72,12 @@ struct DrawItem { > }; > > #define IS_DRAW_ITEM(item) ((item)->type == TREE_ITEM_TYPE_DRAWABLE) > -#define DRAW_ITEM(item) ((DrawItem*)(item)) > +#define DRAW_ITEM(item) SPICE_CONTAINEROF(item, DrawItem, base) > > static inline int is_opaque_item(TreeItem *item) > { > return item->type == TREE_ITEM_TYPE_CONTAINER || > - (IS_DRAW_ITEM(item) && ((DrawItem *)item)->effect == > QXL_EFFECT_OPAQUE); > + (IS_DRAW_ITEM(item) && DRAW_ITEM(item)->effect == QXL_EFFECT_OPAQUE); > } > > void tree_item_dump (TreeItem *item); _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/spice-devel