On Tue, Feb 08, 2011 at 10:00:43PM +0100, Marc-André Lureau wrote:
> Even if the function name is explicit, I would rather see:
>
> cursor_channel_send_item(CursorChannel *cursor_channel, ...) {
> RedChannel *channel = (CursorChannel *)cursor_channel;
You meant (RedChannel *)cursor_channel? so that's what CONTAINEROF is
exactly for. CONTAINEROF(cursor_channel, RedChannel, base)
CONTAINEROF takes some getting used to, it's (pointer, type, field),
once you get the hang of it the type checking is very convenient.
> }
>
> to avoid downcasting, perhaps it's matter of taste, but I feel better
> with upcasts.
Yes, I don't think it's taste only, it makes sense. I basically try
to keep the signatures as explicit as possible (no casts if you can
avoid them), the exceptions are when you have an interface (like RedChannel).
>
> On Mon, Feb 7, 2011 at 7:20 PM, Alon Levy <al...@redhat.com> wrote:
> > Split from cursor_channel_push
> > ---
> > server/red_worker.c | 87
> > ++++++++++++++++++++++++++------------------------
> > 1 files changed, 45 insertions(+), 42 deletions(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index e8849a7..1574b99 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -8407,53 +8407,56 @@ static void display_channel_push(RedWorker *worker)
> > }
> > }
> >
> > +static void cursor_channel_send_item(RedChannel *channel, PipeItem
> > *pipe_item)
> > +{
> > + CursorChannel *cursor_channel = (CursorChannel
> > *)red_ref_channel(channel);
> > + red_channel_reset_send_data(channel);
> > + switch (pipe_item->type) {
> > + case PIPE_ITEM_TYPE_CURSOR:
> > + red_send_cursor(cursor_channel, (CursorItem *)pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_LOCAL_CURSOR:
> > + red_send_local_cursor(cursor_channel, (LocalCursor *)pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_INVAL_ONE:
> > + red_cursor_send_inval(cursor_channel, (CacheItem *)pipe_item);
> > + free(pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_VERB:
> > + red_send_verb(channel, ((VerbItem*)pipe_item)->verb);
> > + free(pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_MIGRATE:
> > + red_printf("PIPE_ITEM_TYPE_MIGRATE");
> > + cursor_channel_send_migrate(cursor_channel);
> > + free(pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_SET_ACK:
> > + red_send_set_ack(channel);
> > + free(pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_CURSOR_INIT:
> > + red_reset_cursor_cache(cursor_channel);
> > + red_send_cursor_init(cursor_channel);
> > + free(pipe_item);
> > + break;
> > + case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
> > + red_reset_cursor_cache(cursor_channel);
> > + red_send_verb(channel, SPICE_MSG_CURSOR_INVAL_ALL);
> > + free(pipe_item);
> > + break;
> > + default:
> > + red_error("invalid pipe item type");
> > + }
> > + red_unref_channel(channel);
> > +}
> > +
> > static void cursor_channel_push(RedWorker *worker)
> > {
> > PipeItem *pipe_item;
> >
> > while ((pipe_item = red_pipe_get((RedChannel
> > *)worker->cursor_channel))) {
> > - CursorChannel *cursor_channel;
> > -
> > - cursor_channel = (CursorChannel *)red_ref_channel((RedChannel
> > *)worker->cursor_channel);
> > - red_channel_reset_send_data((RedChannel*)cursor_channel);
> > - switch (pipe_item->type) {
> > - case PIPE_ITEM_TYPE_CURSOR:
> > - red_send_cursor(cursor_channel, (CursorItem *)pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_LOCAL_CURSOR:
> > - red_send_local_cursor(cursor_channel, (LocalCursor
> > *)pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_INVAL_ONE:
> > - red_cursor_send_inval(cursor_channel, (CacheItem *)pipe_item);
> > - free(pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_VERB:
> > - red_send_verb((RedChannel *)cursor_channel,
> > ((VerbItem*)pipe_item)->verb);
> > - free(pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_MIGRATE:
> > - red_printf("PIPE_ITEM_TYPE_MIGRATE");
> > - cursor_channel_send_migrate(cursor_channel);
> > - free(pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_SET_ACK:
> > - red_send_set_ack((RedChannel *)cursor_channel);
> > - free(pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_CURSOR_INIT:
> > - red_reset_cursor_cache(cursor_channel);
> > - red_send_cursor_init(cursor_channel);
> > - free(pipe_item);
> > - break;
> > - case PIPE_ITEM_TYPE_INVAL_CURSOR_CACHE:
> > - red_reset_cursor_cache(cursor_channel);
> > - red_send_verb((RedChannel *)cursor_channel,
> > SPICE_MSG_CURSOR_INVAL_ALL);
> > - free(pipe_item);
> > - break;
> > - default:
> > - red_error("invalid pipe item type");
> > - }
> > - red_unref_channel((RedChannel *)cursor_channel);
> > + cursor_channel_send_item((RedChannel *)worker->cursor_channel,
> > pipe_item);
> > }
> > }
> >
> > --
> > 1.7.4
> >
> > _______________________________________________
> > Spice-devel mailing list
> > Spice-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/spice-devel
> >
>
>
>
> --
> Marc-André Lureau
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
