>
> ACK. Two minor comments below.
>
>
> On Tue, 2015-10-27 at 19:19 +0000, Frediano Ziglio wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > Signed-off-by: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> > Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> > ---
> > 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
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel