> -----Original Message-----
> From: wayland-devel
> [mailto:[email protected]] On Behalf Of Pekka
> Paalanen
> Sent: Friday, August 14, 2015 9:44 PM
> To: Nobuhiko Tanibata
> Cc: [email protected]; [email protected]
> Subject: Re: [PATCH] ivi-shell: bugfix, list of surfaces on a layer are
> cumulated when set render order is called several time in one commitchanges.
>
> On Wed, 5 Aug 2015 16:00:57 +0900
> Nobuhiko Tanibata <[email protected]> wrote:
>
> > The final list of surfaces of set render order shall be applied. So
> > link of surfaces and list of surfaces in a layer shall be initialized.
> > And then the order of surfaces shall be restructured.
> >
> > Signed-off-by: Nobuhiko Tanibata <[email protected]>
> > ---
> > ivi-shell/ivi-layout.c | 14 +++++++-------
> > 1 file changed, 7 insertions(+), 7 deletions(-)
> >
> > diff --git a/ivi-shell/ivi-layout.c b/ivi-shell/ivi-layout.c index
> > bb175b0..2b61ff2 100644
> > --- a/ivi-shell/ivi-layout.c
> > +++ b/ivi-shell/ivi-layout.c
> > @@ -2082,14 +2082,14 @@ ivi_layout_layer_set_render_order(struct
> ivi_layout_layer *ivilayer,
> > return IVI_FAILED;
> > }
> >
> > - if (pSurface == NULL) {
> > - wl_list_for_each_safe(ivisurf, next,
> &ivilayer->pending.surface_list, pending.link) {
> > - if (!wl_list_empty(&ivisurf->pending.link)) {
> > -
> wl_list_remove(&ivisurf->pending.link);
> > - }
> > + wl_list_for_each_safe(ivisurf, next,
> > + &ivilayer->pending.surface_list,
> pending.link) {
> > + wl_list_init(&ivisurf->pending.link);
> > + }
> >
> > - wl_list_init(&ivisurf->pending.link);
> > - }
> > + wl_list_init(&ivilayer->pending.surface_list);
>
> Hi,
>
> heh, I don't recall seeing this code pattern before. It looks fragile or
> even dangerous, because it is init'ing a link that is part of a list, and
> doing that while traversing that list. However, I think it is safe in this
> case, because:
>
> - wl_list_for_each_safe protects against removal of the current item by
> fetching the pointer to the next item before-hand, so init'ing rather
> than removing the current item is still ok, and
>
> - the whole list is always processed through, and finally the list head
> is init'd, so all involved pointers are reset.
>
> I've been using another pattern, e.g. src/rpi-renderer.c:1768
>
> while (!wl_list_empty(&output->view_cleanup_list)) {
> view = container_of(output->view_cleanup_list.next,
> struct rpir_view, link);
> rpir_view_destroy(view);
> }
>
> I'm not sure if we should prefer one or the other, because I'm obviously
> biased in my judgement. :-)
>
> The latter one does not involve temporarily broken list structures...
[ntanibata] Hi Pekka-san,
I will use later one to send it as v2. Thank you!
Nobuhiko Tanibata
>
> > +
> > + if (pSurface == NULL || number ==0) {
> > ivilayer->event_mask |= IVI_NOTIFICATION_REMOVE;
> > return IVI_SUCCEEDED;
> > }
>
> Reviewed-by: Pekka Paalanen <[email protected]>
>
>
> Thanks,
> pq
_______________________________________________
wayland-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/wayland-devel
