On Fri, Nov 20, 2015 at 12:17 PM, Frediano Ziglio <fziglio@xxxxxxxxxx> wrote: > From: Marc-André Lureau <marcandre.lureau@xxxxxxxxx> > > --- > server/dcc.c | 124 +++++++++++++++++++++++++++++++++++++ > server/dcc.h | 3 + > server/display-channel.h | 7 +++ > server/red_worker.c | 158 ++++------------------------------------------- > 4 files changed, 147 insertions(+), 145 deletions(-) > > diff --git a/server/dcc.c b/server/dcc.c > index fbaf386..eccaa77 100644 > --- a/server/dcc.c > +++ b/server/dcc.c > @@ -1390,3 +1390,127 @@ int dcc_handle_migrate_data(DisplayChannelClient *dcc, uint32_t size, void *mess > red_channel_client_ack_zero_messages_window(RED_CHANNEL_CLIENT(dcc)); > return TRUE; > } > + > +static void image_item_unref(ImageItem *item) > +{ > + if (--item->refs != 0) > + return; > + > + free(item); > +} > + > +static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item) > +{ > + if (--item->refs != 0) > + return; > + > + display_channel_drawable_unref(display, item->drawable); > + free(item->rects); > + free(item); > +} > + > +static void release_item_after_push(DisplayChannelClient *dcc, PipeItem *item) > +{ > + DisplayChannel *display = DCC_TO_DC(dcc); > + > + switch (item->type) { > + case PIPE_ITEM_TYPE_DRAW: > + drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item)); > + break; > + case PIPE_ITEM_TYPE_STREAM_CLIP: > + stream_clip_item_unref(dcc, (StreamClipItem *)item); > + break; > + case PIPE_ITEM_TYPE_UPGRADE: > + upgrade_item_unref(display, (UpgradeItem *)item); > + break; > + case PIPE_ITEM_TYPE_IMAGE: > + image_item_unref((ImageItem *)item); > + break; > + case PIPE_ITEM_TYPE_VERB: > + free(item); > + break; > + case PIPE_ITEM_TYPE_MONITORS_CONFIG: { > + MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item, > + MonitorsConfigItem, pipe_item); > + monitors_config_unref(monconf_item->monitors_config); > + free(item); > + break; > + } > + default: > + spice_critical("invalid item type"); > + } > +} > + > +// TODO: share code between before/after_push since most of the items need the same > +// release > +static void release_item_before_push(DisplayChannelClient *dcc, PipeItem *item) > +{ > + DisplayChannel *display = DCC_TO_DC(dcc); > + > + spice_debug("item.type: %d", item->type); > + switch (item->type) { > + case PIPE_ITEM_TYPE_DRAW: { > + DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item); > + ring_remove(&dpi->base); > + drawable_pipe_item_unref(dpi); > + break; > + } > + case PIPE_ITEM_TYPE_STREAM_CREATE: { > + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item); > + stream_agent_unref(display, agent); > + break; > + } > + case PIPE_ITEM_TYPE_STREAM_CLIP: > + stream_clip_item_unref(dcc, (StreamClipItem *)item); > + break; > + case PIPE_ITEM_TYPE_STREAM_DESTROY: { > + StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item); > + stream_agent_unref(display, agent); > + break; > + } > + case PIPE_ITEM_TYPE_UPGRADE: > + upgrade_item_unref(display, (UpgradeItem *)item); > + break; > + case PIPE_ITEM_TYPE_IMAGE: > + image_item_unref((ImageItem *)item); > + break; > + case PIPE_ITEM_TYPE_CREATE_SURFACE: { > + SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem, > + pipe_item); > + free(surface_create); > + break; > + } > + case PIPE_ITEM_TYPE_DESTROY_SURFACE: { > + SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item, SurfaceDestroyItem, > + pipe_item); > + free(surface_destroy); > + break; > + } > + case PIPE_ITEM_TYPE_MONITORS_CONFIG: { > + MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item, > + MonitorsConfigItem, pipe_item); > + monitors_config_unref(monconf_item->monitors_config); > + free(item); > + break; > + } > + case PIPE_ITEM_TYPE_INVAL_ONE: > + case PIPE_ITEM_TYPE_VERB: > + case PIPE_ITEM_TYPE_MIGRATE_DATA: > + case PIPE_ITEM_TYPE_PIXMAP_SYNC: > + case PIPE_ITEM_TYPE_PIXMAP_RESET: > + case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE: > + case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT: > + free(item); > + break; > + default: > + spice_critical("invalid item type"); > + } > +} > + > +void dcc_release_item(DisplayChannelClient *dcc, PipeItem *item, int item_pushed) > +{ > + if (item_pushed) > + release_item_after_push(dcc, item); > + else > + release_item_before_push(dcc, item); > +} > diff --git a/server/dcc.h b/server/dcc.h > index 9da85ab..13d8c82 100644 > --- a/server/dcc.h > +++ b/server/dcc.h > @@ -194,6 +194,9 @@ void dcc_add_drawable (DisplayCha > void dcc_add_drawable_after (DisplayChannelClient *dcc, > Drawable *drawable, > PipeItem *pos); > +void dcc_release_item (DisplayChannelClient *dcc, > + PipeItem *item, > + int item_pushed); > > typedef struct compress_send_data_t { > void* comp_buf; > diff --git a/server/display-channel.h b/server/display-channel.h > index 4a50912..b49cec9 100644 > --- a/server/display-channel.h > +++ b/server/display-channel.h > @@ -242,6 +242,13 @@ typedef struct SurfaceDestroyItem { > PipeItem pipe_item; > } SurfaceDestroyItem; > > +typedef struct UpgradeItem { > + PipeItem base; > + int refs; > + Drawable *drawable; > + SpiceClipRects *rects; > +} UpgradeItem; > + > > void display_channel_set_stream_video (DisplayChannel *display, > int stream_video); > diff --git a/server/red_worker.c b/server/red_worker.c > index 10b6c00..3873da4 100644 > --- a/server/red_worker.c > +++ b/server/red_worker.c > @@ -85,13 +85,6 @@ struct SpiceWatch { > > #define MAX_PIPE_SIZE 50 > > -typedef struct UpgradeItem { > - PipeItem base; > - int refs; > - Drawable *drawable; > - SpiceClipRects *rects; > -} UpgradeItem; > - > struct RedWorker { > pthread_t thread; > clockid_t clockid; > @@ -151,10 +144,6 @@ static void red_update_area_till(DisplayChannel *display, const SpiceRect *area, > Drawable *last); > static inline void display_begin_send_message(RedChannelClient *rcc); > static int red_display_free_some_independent_glz_drawables(DisplayChannelClient *dcc); > -static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc, > - PipeItem *item); > -static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc, > - PipeItem *item); > static void red_create_surface(DisplayChannel *display, uint32_t surface_id, uint32_t width, > uint32_t height, int32_t stride, uint32_t format, > void *line_0, int data_is_valid, int send_client); > @@ -213,23 +202,6 @@ void red_pipes_remove_drawable(Drawable *drawable) > } > } > > -static void release_image_item(ImageItem *item) > -{ > - if (!--item->refs) { > - free(item); > - } > -} > - > -static void upgrade_item_unref(DisplayChannel *display, UpgradeItem *item) > -{ > - if (--item->refs != 0) > - return; > - > - display_channel_drawable_unref(display, item->drawable); > - free(item->rects); > - free(item); > -} > - > static uint8_t *common_alloc_recv_buf(RedChannelClient *rcc, uint16_t type, uint32_t size) > { > CommonChannel *common = SPICE_CONTAINEROF(rcc->channel, CommonChannel, base); > @@ -3960,7 +3932,7 @@ static void red_marshall_stream_activate_report(RedChannelClient *rcc, > spice_marshall_msg_display_stream_activate_report(base_marshaller, &msg); > } > > -static void display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item) > +static void send_item(RedChannelClient *rcc, PipeItem *pipe_item) This rename is not relevant for this patch ... > { > SpiceMarshaller *m = red_channel_client_get_marshaller(rcc); > DisplayChannelClient *dcc = RCC_TO_DCC(rcc); > @@ -4041,7 +4013,7 @@ static void display_channel_send_item(RedChannelClient *rcc, PipeItem *pipe_item > spice_error("invalid pipe item type"); > } > > - display_channel_client_release_item_before_push(dcc, pipe_item); > + dcc_release_item(dcc, pipe_item, FALSE); > > // a message is pending > if (red_channel_client_send_message_pending(rcc)) { > @@ -4059,7 +4031,7 @@ static inline void red_push(RedWorker *worker) > } > } > > -static void display_channel_client_on_disconnect(RedChannelClient *rcc) > +static void on_disconnect(RedChannelClient *rcc) Same here ... > { > DisplayChannel *display; > DisplayChannelClient *dcc = RCC_TO_DCC(rcc); > @@ -4335,7 +4307,7 @@ static inline void flush_all_qxl_commands(RedWorker *worker) > flush_cursor_commands(worker); > } > > -static int display_channel_handle_migrate_mark(RedChannelClient *rcc) > +static int handle_migrate_flush_mark(RedChannelClient *rcc) Same here ... > { > DisplayChannel *display_channel = SPICE_CONTAINEROF(rcc->channel, DisplayChannel, common.base); > RedChannel *channel = RED_CHANNEL(display_channel); > @@ -4540,7 +4512,7 @@ RedChannel *red_worker_new_channel(RedWorker *worker, int size, > return channel; > } > > -static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item) > +static void hold_item(RedChannelClient *rcc, PipeItem *item) Same here ... > { > spice_assert(item); > switch (item->type) { > @@ -4561,116 +4533,12 @@ static void display_channel_hold_pipe_item(RedChannelClient *rcc, PipeItem *item > } > } > > -static void display_channel_client_release_item_after_push(DisplayChannelClient *dcc, > - PipeItem *item) > -{ > - DisplayChannel *display = DCC_TO_DC(dcc); > - > - switch (item->type) { > - case PIPE_ITEM_TYPE_DRAW: > - drawable_pipe_item_unref(SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item)); > - break; > - case PIPE_ITEM_TYPE_STREAM_CLIP: > - stream_clip_item_unref(dcc, (StreamClipItem *)item); > - break; > - case PIPE_ITEM_TYPE_UPGRADE: > - upgrade_item_unref(display, (UpgradeItem *)item); > - break; > - case PIPE_ITEM_TYPE_IMAGE: > - release_image_item((ImageItem *)item); > - break; > - case PIPE_ITEM_TYPE_VERB: > - free(item); > - break; > - case PIPE_ITEM_TYPE_MONITORS_CONFIG: { > - MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item, > - MonitorsConfigItem, pipe_item); > - monitors_config_unref(monconf_item->monitors_config); > - free(item); > - break; > - } > - default: > - spice_critical("invalid item type"); > - } > -} > - > -// TODO: share code between before/after_push since most of the items need the same > -// release > -static void display_channel_client_release_item_before_push(DisplayChannelClient *dcc, > - PipeItem *item) > -{ > - DisplayChannel *display = DCC_TO_DC(dcc); > - > - switch (item->type) { > - case PIPE_ITEM_TYPE_DRAW: { > - DrawablePipeItem *dpi = SPICE_CONTAINEROF(item, DrawablePipeItem, dpi_pipe_item); > - ring_remove(&dpi->base); > - drawable_pipe_item_unref(dpi); > - break; > - } > - case PIPE_ITEM_TYPE_STREAM_CREATE: { > - StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, create_item); > - stream_agent_unref(display, agent); > - break; > - } > - case PIPE_ITEM_TYPE_STREAM_CLIP: > - stream_clip_item_unref(dcc, (StreamClipItem *)item); > - break; > - case PIPE_ITEM_TYPE_STREAM_DESTROY: { > - StreamAgent *agent = SPICE_CONTAINEROF(item, StreamAgent, destroy_item); > - stream_agent_unref(display, agent); > - break; > - } > - case PIPE_ITEM_TYPE_UPGRADE: > - upgrade_item_unref(display, (UpgradeItem *)item); > - break; > - case PIPE_ITEM_TYPE_IMAGE: > - release_image_item((ImageItem *)item); > - break; > - case PIPE_ITEM_TYPE_CREATE_SURFACE: { > - SurfaceCreateItem *surface_create = SPICE_CONTAINEROF(item, SurfaceCreateItem, > - pipe_item); > - free(surface_create); > - break; > - } > - case PIPE_ITEM_TYPE_DESTROY_SURFACE: { > - SurfaceDestroyItem *surface_destroy = SPICE_CONTAINEROF(item, SurfaceDestroyItem, > - pipe_item); > - free(surface_destroy); > - break; > - } > - case PIPE_ITEM_TYPE_MONITORS_CONFIG: { > - MonitorsConfigItem *monconf_item = SPICE_CONTAINEROF(item, > - MonitorsConfigItem, pipe_item); > - monitors_config_unref(monconf_item->monitors_config); > - free(item); > - break; > - } > - case PIPE_ITEM_TYPE_INVAL_ONE: > - case PIPE_ITEM_TYPE_VERB: > - case PIPE_ITEM_TYPE_MIGRATE_DATA: > - case PIPE_ITEM_TYPE_PIXMAP_SYNC: > - case PIPE_ITEM_TYPE_PIXMAP_RESET: > - case PIPE_ITEM_TYPE_INVAL_PALETTE_CACHE: > - case PIPE_ITEM_TYPE_STREAM_ACTIVATE_REPORT: > - free(item); > - break; > - default: > - spice_critical("invalid item type"); > - } > -} > - > -static void display_channel_release_item(RedChannelClient *rcc, PipeItem *item, int item_pushed) > +static void release_item(RedChannelClient *rcc, PipeItem *item, int item_pushed) Same here ... > { > DisplayChannelClient *dcc = RCC_TO_DCC(rcc); > > - spice_assert(item); > - if (item_pushed) { > - display_channel_client_release_item_after_push(dcc, item); > - } else { > - spice_debug("not pushed (%d)", item->type); > - display_channel_client_release_item_before_push(dcc, item); > - } > + spice_return_if_fail(item != NULL); > + dcc_release_item(dcc, item, item_pushed); > } > > static void image_surface_init(DisplayChannel *display) > @@ -4687,11 +4555,11 @@ static void display_channel_create(RedWorker *worker, int migrate, int stream_vi > { > DisplayChannel *display_channel; > ChannelCbs cbs = { > - .on_disconnect = display_channel_client_on_disconnect, > - .send_item = display_channel_send_item, > - .hold_item = display_channel_hold_pipe_item, > - .release_item = display_channel_release_item, > - .handle_migrate_flush_mark = display_channel_handle_migrate_mark, > + .on_disconnect = on_disconnect, > + .send_item = send_item, > + .hold_item = hold_item, > + .release_item = release_item, > + .handle_migrate_flush_mark = handle_migrate_flush_mark, Same here ... > .handle_migrate_data = handle_migrate_data, > .handle_migrate_data_get_serial = handle_migrate_data_get_serial > }; > -- > 2.4.3 > > _______________________________________________ > Spice-devel mailing list > Spice-devel@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/spice-devel ACK with these changes ... -- Fabiano Fidêncio _______________________________________________ Spice-devel mailing list Spice-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/spice-devel