>
> On Mon, 2015-11-23 at 17:01 +0000, Frediano Ziglio wrote:
> > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx>
> >
> > ---
> > server/red_worker.c | 65
> > +++++++++++++++++++++++++---------------------------
> > -
> > 1 file changed, 31 insertions(+), 34 deletions(-)
> >
> > diff --git a/server/red_worker.c b/server/red_worker.c
> > index f64befa..e0fd6e5 100644
> > --- a/server/red_worker.c
> > +++ b/server/red_worker.c
> > @@ -4889,34 +4889,6 @@ static void handle_new_display_channel(RedWorker
> > *worker, RedClient *client, Red
> > dcc_start(dcc);
> > }
> >
> > -static void red_connect_cursor(RedWorker *worker, RedClient *client,
> > RedsStream *stream,
> > - int migrate,
> > - uint32_t *common_caps, int num_common_caps,
> > - uint32_t *caps, int num_caps)
> > -{
> > - CursorChannel *channel;
> > - CursorChannelClient *ccc;
> > -
> > - spice_return_if_fail(worker->cursor_channel != NULL);
> > -
> > - channel = worker->cursor_channel;
> > - spice_info("add cursor channel client");
> > - ccc = cursor_channel_client_new(channel, client, stream,
> > - migrate,
> > - common_caps, num_common_caps,
> > - caps, num_caps);
> > - spice_return_if_fail(ccc != NULL);
> > -
> > - RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> > - red_channel_client_ack_zero_messages_window(rcc);
> > - red_channel_client_push_set_ack(rcc);
> > -
> > - // TODO: why do we check for context.canvas? defer this to after
> > display
> > cc is connected
> > - // and test it's canvas? this is just a test to see if there is an
> > active
> > renderer?
> > - if (display_channel_surface_has_canvas(worker->display_channel, 0))
> > - cursor_channel_init(channel, ccc);
> > -}
> > -
> > static void region_to_qxlrects(QRegion *region, QXLRect *qxl_rects,
> > uint32_t
> > num_rects)
> > {
> > SpiceRect *rects;
> > @@ -5422,19 +5394,44 @@ static void handle_dev_monitors_config_async(void
> > *opaque, void *payload)
> > red_worker_push_monitors_config(worker);
> > }
> >
> > +static void cursor_connect(RedWorker *worker, RedClient *client,
> > RedsStream
> > *stream,
> > + int migrate,
> > + uint32_t *common_caps, int num_common_caps,
> > + uint32_t *caps, int num_caps)
> > +{
> > + CursorChannel *channel = worker->cursor_channel;
> > + CursorChannelClient *ccc;
> > +
> > + spice_return_if_fail(worker->cursor_channel != NULL);
> > +
> > + spice_info("add cursor channel client");
> > + ccc = cursor_channel_client_new(channel, client, stream,
> > + migrate,
> > + common_caps, num_common_caps,
> > + caps, num_caps);
> > + spice_return_if_fail(ccc != NULL);
> > +
> > + RedChannelClient *rcc = RED_CHANNEL_CLIENT(ccc);
> > + red_channel_client_ack_zero_messages_window(rcc);
> > + red_channel_client_push_set_ack(rcc);
> > +
> > + // TODO: why do we check for context.canvas? defer this to after
> > display
> > cc is connected
> > + // and test it's canvas? this is just a test to see if there is an
> > active
> > renderer?
> > + if (display_channel_surface_has_canvas(worker->display_channel, 0))
> > + cursor_channel_init(channel, ccc);
> > +}
> > +
> > /* TODO: special, perhaps use another dispatcher? */
> > static void handle_dev_cursor_connect(void *opaque, void *payload)
> > {
> > RedWorkerMessageCursorConnect *msg = payload;
> > RedWorker *worker = opaque;
> > - RedsStream *stream = msg->stream;
> > - RedClient *client = msg->client;
> > - int migration = msg->migration;
> >
> > spice_info("cursor connect");
> > - red_connect_cursor(worker, client, stream, migration,
> > - msg->common_caps, msg->num_common_caps,
> > - msg->caps, msg->num_caps);
> > + cursor_connect(worker,
> > + msg->client, msg->stream, msg->migration,
> > + msg->common_caps, msg->num_common_caps,
> > + msg->caps, msg->num_caps);
> > free(msg->caps);
> > free(msg->common_caps);
> > }
>
>
> I don't really see much point to most of this commit. But I don't see much
> reason *not* to do it either... Basically all it does is:
>
> - renames the function
> - moves the definition to a different place in the source file (not sure why)
> - avoids creating local variables in the caller.
>
> So I'm pretty ambivalent. But I'll ACK it to avoid creating conflicts further
> in
> the branch.
>
> Jonathon
>
In the previous version the cursor_channel_client_new was also sending some
initial message so cursor_connect was shorter. However I moved these lines
back.
Perhaps I should add me as signed-off and change the commit message.
Wait... why the patch is just moving a function from one place to another
in the same file.. is confusing and also the function will be moved to a
different file later... I'll change it.
NACK for now.
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel