[PATCH libinput 1/5] util: allow list_remove() on a NULL node

[Peter Hutterer]

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 <peter.hutterer at who-t.net>
> >---
> >  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 <hdegoede at redhat.com>

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