On Wed, 2015-10-28 at 09:01 -0400, Frediano Ziglio wrote:
> >
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > Do not use static allocate space but handle dynamically
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > server/cursor-channel.h | 9 --------
> > server/red_worker.c | 56
> > ++++++++++---------------------------------------
> > 2 files changed, 11 insertions(+), 54 deletions(-)
> >
> > diff --git a/server/cursor-channel.h b/server/cursor-channel.h
> > index 48b2c71..4654bcb 100644
> > --- a/server/cursor-channel.h
> > +++ b/server/cursor-channel.h
> > @@ -64,15 +64,6 @@ typedef struct CursorChannel {
> > #endif
> > } CursorChannel;
> >
> > -typedef struct _CursorItem _CursorItem;
> > -
> > -struct _CursorItem {
> > - union {
> > - CursorItem cursor_item;
> > - _CursorItem *next;
> > - } u;
> > -};
> > -
> > G_STATIC_ASSERT(sizeof(CursorItem) <=
> > QXL_CURSUR_DEVICE_DATA_SIZE);
> >
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index a55e57a..631ea11 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -646,9 +646,6 @@ typedef struct RedWorker {
> > _Drawable drawables[NUM_DRAWABLES];
> > _Drawable *free_drawables;
> >
> > - _CursorItem cursor_items[NUM_CURSORS];
> > - _CursorItem *free_cursor_items;
> > -
> > RedMemSlotInfo mem_slots;
> >
> > ImageCache image_cache;
> > @@ -4234,8 +4231,6 @@ static void red_update_area(RedWorker
> > *worker, const
> > SpiceRect *area, int surfac
> > validate_area(worker, area, surface_id);
> > }
> >
> > -static inline void free_cursor_item(RedWorker *worker, CursorItem
> > *item);
> > -
> > static void red_release_cursor(RedWorker *worker, CursorItem
> > *cursor)
> > {
> > if (!--cursor->refs) {
> > @@ -4246,9 +4241,10 @@ static void red_release_cursor(RedWorker
> > *worker,
> > CursorItem *cursor)
> > release_info_ext.group_id = cursor->group_id;
> > release_info_ext.info = cursor_cmd->release_info;
> > worker->qxl->st->qif->release_resource(worker->qxl,
> > release_info_ext);
> > - free_cursor_item(worker, cursor);
> > red_put_cursor_cmd(cursor_cmd);
> > free(cursor_cmd);
> > +
> > + g_slice_free(CursorItem, cursor);
> > }
> > }
> >
> > @@ -4261,52 +4257,23 @@ static void red_set_cursor(RedWorker
> > *worker,
> > CursorItem *cursor)
> > worker->cursor = cursor;
> > }
> >
> > -#ifdef DEBUG_CURSORS
> > -static int _cursor_count = 0;
> > -#endif
> > -
> > -static inline CursorItem *alloc_cursor_item(RedWorker *worker)
> > -{
> > - CursorItem *cursor;
> > -
> > - if (!worker->free_cursor_items) {
> > - return NULL;
> > - }
> > -#ifdef DEBUG_CURSORS
> > - --_cursor_count;
> > -#endif
> > - cursor = &worker->free_cursor_items->u.cursor_item;
> > - worker->free_cursor_items = worker->free_cursor_items->u.next;
> > - return cursor;
> > -}
> > -
> > -static inline void free_cursor_item(RedWorker *worker, CursorItem
> > *item)
> > +static inline CursorItem *alloc_cursor_item(void)
> > {
> > - ((_CursorItem *)item)->u.next = worker->free_cursor_items;
> > - worker->free_cursor_items = (_CursorItem *)item;
> > -#ifdef DEBUG_CURSORS
> > - ++_cursor_count;
> > - spice_assert(_cursor_count <= NUM_CURSORS);
> > -#endif
> > -}
> > + CursorItem *cursor_item;
> >
> > -static void cursor_items_init(RedWorker *worker)
> > -{
> > - int i;
> > + cursor_item = g_slice_new0(CursorItem);
> > + cursor_item->refs = 1;
> >
> > - worker->free_cursor_items = NULL;
> > - for (i = 0; i < NUM_CURSORS; i++) {
> > - free_cursor_item(worker, &worker
> > ->cursor_items[i].u.cursor_item);
> > - }
> > + return cursor_item;
> > }
> >
> > -static CursorItem *get_cursor_item(RedWorker *worker, RedCursorCmd
> > *cmd,
> > uint32_t group_id)
> > +static CursorItem *get_cursor_item(RedCursorCmd *cmd, uint32_t
> > group_id)
> > {
> > CursorItem *cursor_item;
> >
> > - spice_warn_if(!(cursor_item = alloc_cursor_item(worker)));
> > + spice_return_val_if_fail(cmd != NULL, NULL);
> > + spice_warn_if(!(cursor_item = alloc_cursor_item()));
> >
>
> Next instruction are going to core if the pointer is ULL, perhaps
> would
> be better a spice_error or spice_critical.
It seems to me that we should instead replace spice_warn_if() with
spice_return_val_if_fail(cursor_item, NULL). get_cursor_item() is only
used from qxl_process_cursor(), so we would only need to change that
function to handle a NULL return from this function. Then we will have
a useful warning message on the debug output and won't abort the qemu
process when this error occurs. But this is a pre-existing issue that
can be handled in a separate patch.
>
> > - cursor_item->refs = 1;
> > cursor_item->group_id = group_id;
> > cursor_item->red_cursor = cmd;
> >
> > @@ -4350,7 +4317,7 @@ static void qxl_process_cursor(RedWorker
> > *worker,
> > RedCursorCmd *cursor_cmd, uint
> > CursorItem *cursor_item;
> > int cursor_show = FALSE;
> >
> > - cursor_item = get_cursor_item(worker, cursor_cmd, group_id);
> > + cursor_item = get_cursor_item(cursor_cmd, group_id);
> >
> > switch (cursor_cmd->type) {
> > case QXL_CURSOR_SET:
> > @@ -11629,7 +11596,6 @@ RedWorker* red_worker_new(QXLInstance *qxl,
> > RedDispatcher *red_dispatcher)
> > image_cache_init(&worker->image_cache);
> > image_surface_init(worker);
> > drawables_init(worker);
> > - cursor_items_init(worker);
> > red_init_streams(worker);
> > stat_init(&worker->add_stat, add_stat_name);
> > stat_init(&worker->exclude_stat, exclude_stat_name);
> > --
> > 2.4.3
> >
> >
>
> Frediano
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel