On 10/28/2015 06:07 PM, Frediano Ziglio wrote:
From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> Also fix warning due to unexpected pipe item type The specific item type that was not being handled was PIPE_ITEM_TYPE_INVAL_ONE (#102). This item type is used by the cursor channel, but the analogous item for the display channel is PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE. Use this value instead. The exact warning follows: (/usr/bin/qemu-kvm:24458): Spice-Warning **: ../../server/dcc-send.c:2442:dcc_send_item: should not be reached (/usr/bin/qemu-kvm:24458): Spice-CRITICAL **: ../../server/dcc.c:1595:release_item_before_push: invalid item type Author: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> Date: Thu Feb 27 19:38:58 2014 +0200 --- server/cache_item.tmpl.c | 4 +- server/cursor-channel.c | 219 ++++++++++++++++++++++++++--------------------- server/cursor-channel.h | 27 +++--- server/red_dispatcher.c | 1 + server/red_worker.c | 200 +++++++++++++++++++------------------------ server/red_worker.h | 65 +++++--------- 6 files changed, 255 insertions(+), 261 deletions(-) diff --git a/server/cache_item.tmpl.c b/server/cache_item.tmpl.c index dc314c0..ad2b579 100644 --- a/server/cache_item.tmpl.c +++ b/server/cache_item.tmpl.c @@ -21,6 +21,7 @@ #define CACHE_HASH_KEY CURSOR_CACHE_HASH_KEY #define CACHE_HASH_SIZE CURSOR_CACHE_HASH_SIZE #define CACHE_INVAL_TYPE SPICE_MSG_CURSOR_INVAL_ONE +#define PIPE_ITEM_TYPE PIPE_ITEM_TYPE_INVAL_ONEI would prefer a PIPE_ITEM_TYPE_INVALID macro here. Is not clear which part of the patch is fixing the "unexpected pipe item type".
I think both are ok.
#define FUNC_NAME(name) red_cursor_cache_##name #define VAR_NAME(name) cursor_cache_##name #define CHANNEL CursorChannel @@ -32,6 +33,7 @@ #define CACHE_HASH_KEY PALETTE_CACHE_HASH_KEY #define CACHE_HASH_SIZE PALETTE_CACHE_HASH_SIZE #define CACHE_INVAL_TYPE SPICE_MSG_DISPLAY_INVAL_PALETTE +#define PIPE_ITEM_TYPE PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE
This line fixes the "unexpected pipe item type".
#define FUNC_NAME(name) red_palette_cache_##name #define VAR_NAME(name) palette_cache_##name #define CHANNEL DisplayChannel @@ -78,7 +80,7 @@ static void FUNC_NAME(remove)(CHANNELCLIENT *channel_client, CacheItem *item) channel_client->VAR_NAME(items)--; channel_client->VAR_NAME(available) += item->size; - red_channel_pipe_item_init(&channel->common.base, &item->u.pipe_data, PIPE_ITEM_TYPE_INVAL_ONE); + red_channel_pipe_item_init(&channel->common.base, &item->u.pipe_data, PIPE_ITEM_TYPE); red_channel_client_pipe_add_tail(&channel_client->common.base, &item->u.pipe_data); // for now } diff --git a/server/cursor-channel.c b/server/cursor-channel.c index 7ba4a7d..adf7d78 100644 --- a/server/cursor-channel.c +++ b/server/cursor-channel.c @@ -25,57 +25,64 @@ #include "cache_item.tmpl.c" #undef CLIENT_CURSOR_CACHE -static inline CursorItem *alloc_cursor_item(void) +CursorItem *cursor_item_new(QXLInstance *qxl, RedCursorCmd *cmd, uint32_t group_id) { CursorItem *cursor_item; + spice_return_val_if_fail(cmd != NULL, NULL); + cursor_item = g_slice_new0(CursorItem); + cursor_item->qxl = qxl; cursor_item->refs = 1; + cursor_item->group_id = group_id; + cursor_item->red_cursor = cmd; return cursor_item; } -CursorItem *cursor_item_new(RedCursorCmd *cmd, uint32_t group_id) +CursorItem *cursor_item_ref(CursorItem *item) { - CursorItem *cursor_item; + spice_return_val_if_fail(item != NULL, NULL); + spice_return_val_if_fail(item->refs > 0, NULL);This is detecting a memory error, perhaps a spice_error or a spice_critical should be better.- spice_return_val_if_fail(cmd != NULL, NULL); - spice_warn_if(!(cursor_item = alloc_cursor_item())); - - cursor_item->group_id = group_id; - cursor_item->red_cursor = cmd; + item->refs++; - return cursor_item; + return item; } -void cursor_item_unref(QXLInstance *qxl, CursorItem *cursor) +void cursor_item_unref(CursorItem *item) { - if (!--cursor->refs) { - QXLReleaseInfoExt release_info_ext; - RedCursorCmd *cursor_cmd; - - cursor_cmd = cursor->red_cursor; - release_info_ext.group_id = cursor->group_id; - release_info_ext.info = cursor_cmd->release_info; - qxl->st->qif->release_resource(qxl, release_info_ext); - red_put_cursor_cmd(cursor_cmd); - free(cursor_cmd); - - g_slice_free(CursorItem, cursor); - } + QXLReleaseInfoExt release_info_ext; + RedCursorCmd *cursor_cmd; + + spice_return_if_fail(item != NULL); + + if (--item->refs) + return; + + cursor_cmd = item->red_cursor; + release_info_ext.group_id = item->group_id; + release_info_ext.info = cursor_cmd->release_info; + item->qxl->st->qif->release_resource(item->qxl, release_info_ext); + red_put_cursor_cmd(cursor_cmd); + free(cursor_cmd); + + g_slice_free(CursorItem, item); + }There are a lot of renames cursor_channel -> cursor and cursor -> item (here and in following hunks). Should we remove them ? Which naming should we use ?
I like cursor_channel better than cursor (or maybe even just channel). I like cursor_item better than cursor (or maybe just item). I like a patch that says "move some code" to only move code and not change code that is moved. I think this patch should be split. - Uri. _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel