[Spice-devel] [PATCH 11/11] Get code more typesafe
Frediano Ziglio
fziglio at redhat.com
Fri May 20 13:01:49 UTC 2016
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 at redhat.com>
---
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);
--
2.7.4
More information about the Spice-devel
mailing list