On Tue, Jun 30, 2015 at 02:22:34PM +0200, Hans de Goede wrote: > Hi Peter, > > On 29-06-15 05:49, Peter Hutterer wrote: > >Don't require a list_init() on a node before we can call list_remove on it, > >it > >clutters up the code for little benefit. > > > >Signed-off-by: Peter Hutterer <[email protected]> > >--- > > src/libinput-util.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > >diff --git a/src/libinput-util.c b/src/libinput-util.c > >index 3a9c8db..f19695c 100644 > >--- a/src/libinput-util.c > >+++ b/src/libinput-util.c > >@@ -59,6 +59,9 @@ list_insert(struct list *list, struct list *elm) > > void > > list_remove(struct list *elm) > > { > >+ if (elm->next == NULL && elm->prev == NULL) > >+ return; > >+ > > elm->prev->next = elm->next; > > elm->next->prev = elm->prev; > > elm->next = NULL; > > > > I do not think this is a good idea, most list implementations > people are used to do not allow this and consider a double > remove / del a bug. > > At a minimum this needs to be done in the form of adding a > wrapper called: list_remove_if_not_null() but I would prefer > for you to not make such a change at all, instead I > suggest adding an extra "if (tp->palm.monitor_trackpoint)" > check before the 2 libinput_device_remove_event_listener( > &tp->sendevents.trackpoint_listener) calls to patch 5/5 . > > With those 2 extra checks added patches 2 - 5 are: > > Reviewed-by: Hans de Goede <[email protected]>
right, fair enough. I've dropped this patch now and added the checks as requested to 5/5. Not deviating from the other list implementations is the better option here. Thanks for the review Cheers, Peter _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
