On Fri, 13 Feb 2015 14:01:53 +0800 Jonas Ådahl <[email protected]> wrote:
> Can just use wl_list_for_each_safe instead of dealing with pointers > ourself. > > Signed-off-by: Jonas Ådahl <[email protected]> > --- > desktop-shell/shell.c | 36 +++++++++++++++--------------------- > 1 file changed, 15 insertions(+), 21 deletions(-) > > diff --git a/desktop-shell/shell.c b/desktop-shell/shell.c > index f28fc10..db0c5a9 100644 > --- a/desktop-shell/shell.c > +++ b/desktop-shell/shell.c > @@ -3059,20 +3059,18 @@ destroy_shell_seat(struct wl_listener *listener, void > *data) > struct shell_seat *shseat = > container_of(listener, > struct shell_seat, seat_destroy_listener); > - struct shell_surface *shsurf, *prev = NULL; > + struct shell_surface *shsurf, *next; > > if (shseat->popup_grab.grab.interface == &popup_grab_interface) { > weston_pointer_end_grab(shseat->popup_grab.grab.pointer); > shseat->popup_grab.client = NULL; > > - wl_list_for_each(shsurf, &shseat->popup_grab.surfaces_list, > popup.grab_link) { > + wl_list_for_each_safe(shsurf, next, > + &shseat->popup_grab.surfaces_list, > + popup.grab_link) { > shsurf->popup.shseat = NULL; > - if (prev) { > - wl_list_init(&prev->popup.grab_link); > - } > - prev = shsurf; > + wl_list_init(&shsurf->popup.grab_link); > } > - wl_list_init(&prev->popup.grab_link); Hi, while I can understand that this code very likely works right, I feel a little awkward using wl_list_init() where one would normally use wl_list_remove(). Even though it is somewhat redundant, I would like to see wl_list_remove() followed by wl_list_init(), iff grab_link really needs to be initialized. But, I see the old code already did the awkward thing, so fixing that can be left for another patch. All my comments for the below hunks are just the same. This patch pushed with my R-b. d2c6892..c2b1011 master -> master Thanks, pq > } > > wl_list_remove(&shseat->seat_destroy_listener.link); > @@ -3328,7 +3326,7 @@ popup_grab_end(struct weston_pointer *pointer) > struct shell_seat *shseat = > container_of(grab, struct shell_seat, popup_grab.grab); > struct shell_surface *shsurf; > - struct shell_surface *prev = NULL; > + struct shell_surface *next; > > if (pointer->grab->interface == &popup_grab_interface) { > weston_pointer_end_grab(grab->pointer); > @@ -3336,15 +3334,13 @@ popup_grab_end(struct weston_pointer *pointer) > shseat->popup_grab.grab.interface = NULL; > assert(!wl_list_empty(&shseat->popup_grab.surfaces_list)); > /* Send the popup_done event to all the popups open */ > - wl_list_for_each(shsurf, &shseat->popup_grab.surfaces_list, > popup.grab_link) { > + wl_list_for_each_safe(shsurf, next, > + &shseat->popup_grab.surfaces_list, > + popup.grab_link) { > shell_surface_send_popup_done(shsurf); > shsurf->popup.shseat = NULL; > - if (prev) { > - wl_list_init(&prev->popup.grab_link); > - } > - prev = shsurf; > + wl_list_init(&shsurf->popup.grab_link); > } > - wl_list_init(&prev->popup.grab_link); > wl_list_init(&shseat->popup_grab.surfaces_list); > } > } > @@ -3356,7 +3352,7 @@ touch_popup_grab_end(struct weston_touch *touch) > struct shell_seat *shseat = > container_of(grab, struct shell_seat, popup_grab.touch_grab); > struct shell_surface *shsurf; > - struct shell_surface *prev = NULL; > + struct shell_surface *next; > > if (touch->grab->interface == &touch_popup_grab_interface) { > weston_touch_end_grab(grab->touch); > @@ -3364,15 +3360,13 @@ touch_popup_grab_end(struct weston_touch *touch) > shseat->popup_grab.touch_grab.interface = NULL; > assert(!wl_list_empty(&shseat->popup_grab.surfaces_list)); > /* Send the popup_done event to all the popups open */ > - wl_list_for_each(shsurf, &shseat->popup_grab.surfaces_list, > popup.grab_link) { > + wl_list_for_each_safe(shsurf, next, > + &shseat->popup_grab.surfaces_list, > + popup.grab_link) { > shell_surface_send_popup_done(shsurf); > shsurf->popup.shseat = NULL; > - if (prev) { > - wl_list_init(&prev->popup.grab_link); > - } > - prev = shsurf; > + wl_list_init(&shsurf->popup.grab_link); > } > - wl_list_init(&prev->popup.grab_link); > wl_list_init(&shseat->popup_grab.surfaces_list); > } > } _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
