[Spice-devel] [PATCH 05/11] worker: move some cursor code to cursor-channel.c

[Frediano Ziglio]

> 
> ACK. Two minor comments below.
> 
> 
> On Tue, 2015-10-27 at 19:19 +0000, Frediano Ziglio wrote:
> > From: Marc-André Lureau <marcandre.lureau at gmail.com>
> > 
> > Signed-off-by: Marc-André Lureau <marcandre.lureau at gmail.com>
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/Makefile.am      |   1 +
> >  server/cursor-channel.c | 464 ++++++++++++++++++++++++++++++++++++
> >  server/cursor-channel.h |  26 +++
> >  server/red_worker.c     | 611 +++++---------------------------------
> > ----------
> >  server/red_worker.h     |  92 ++++++++
> >  5 files changed, 635 insertions(+), 559 deletions(-)
> >  create mode 100644 server/cursor-channel.c
> > 
> 
> ....
> 
> > +// TODO: share code between before/after_push since most of the
> > items need the same
> > +// release
> > +static void
> > cursor_channel_client_release_item_before_push(CursorChannelClient
> > *ccc,
> > +                                                           PipeItem
> > *item)
> > +{
> > +    switch (item->type) {
> > +    case PIPE_ITEM_TYPE_CURSOR: {
> > +        CursorPipeItem *cursor_pipe_item = SPICE_CONTAINEROF(item,
> > CursorPipeItem, base);
> > +        put_cursor_pipe_item(ccc, cursor_pipe_item);
> > +        break;
> > +    }
> > +    case PIPE_ITEM_TYPE_INVAL_ONE:
> > +    case PIPE_ITEM_TYPE_VERB:
> > +    case PIPE_ITEM_TYPE_CURSOR_INIT:
> > +    case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
> > +        free(item);
> > +        break;
> > +    default:
> > +        spice_error("invalid pipe item type");
> > +    }
> > +}
> > +
> > +static void
> > cursor_channel_client_release_item_after_push(CursorChannelClient
> > *ccc,
> > +                                                          PipeItem
> > *item)
> > +{
> > +    switch (item->type) {
> > +        case PIPE_ITEM_TYPE_CURSOR: {
> > +            CursorPipeItem *cursor_pipe_item =
> > SPICE_CONTAINEROF(item, CursorPipeItem, base);
> > +            put_cursor_pipe_item(ccc, cursor_pipe_item);
> > +            break;
> > +        }
> > +        default:
> > +            spice_critical("invalid item type");
> 
> 
> This is an existing inconsistency, but here we use spice_critical(),
> and just above in _before_push(), we use spice_error() for the same
> situation. Would probably be good to choose one and be consistent, even
> if they do behave the same with default settings...
> 
> 
> >  
> > +static inline void red_marshall_inval(RedChannelClient *rcc,
> > +                                      SpiceMarshaller
> > *base_marshaller, CacheItem *cach_item)
> > +{
> > +    SpiceMsgDisplayInvalOne inval_one;
> > +
> > +    red_channel_client_init_send_data(rcc, cach_item->inval_type,
> > NULL);
> > +    inval_one.id = *(uint64_t *)&cach_item->id;
> > +
> > +    spice_marshall_msg_cursor_inval_one(base_marshaller,
> > &inval_one);
> > +}
> > +
> 
> 
> Up above, this function was removed from red_worker.c and was added to
> cursor-channel.c. Here it is being added back to red_worker.c again.
> Probably this one can be removed?
> 
> 
> 

Moved in the original place to avoid the remove+insert.

Frediano