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... > + > + if (pSurface == NULL || number ==0) { > ivilayer->event_mask |= IVI_NOTIFICATION_REMOVE; > return IVI_SUCCEEDED; > } Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq
pgp67rMgTaLKk.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
