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

Peter Hutterer peter.hutterer at who-t.net
Mon Jun 29 16:32:32 PDT 2015


On Mon, Jun 29, 2015 at 04:00:51PM -0700, Ping Cheng wrote:
> On Mon, Jun 29, 2015 at 3:09 PM, Peter Hutterer <peter.hutterer at who-t.net>
> wrote:
> 
> > On Mon, Jun 29, 2015 at 12:37:26PM -0700, Bill Spitzak wrote:
> > > On Sun, Jun 28, 2015 at 8:49 PM, Peter Hutterer <
> > peter.hutterer at who-t.net>
> > > wrote:
> > >
> > > >
> > > > +       if (elm->next == NULL && elm->prev == NULL)
> > > > +               return;
> > > > +
> > > >         elm->prev->next = elm->next;
> > > >         elm->next->prev = elm->prev;
> > > >         elm->next = NULL;
> > > >
> > >
> > >  You probably don't need to check both pointers, as the code will crash
> > if
> > > only one of them is NULL.
> >
> > yeah, that's true but obviousness in code is worth a lot. only checking
> > next
> > or prev will make the casual reviewer wonder why we don't check both, so
> > it'd require a comment or generally more brain-power to review than the
> > bleedingly obvious condition.
> 
> 
> I guess Bill meant "||" should be used instead of "&&"? One of the == NULL
> would lead to a crash...

that'd would hide potential memory corruption or other bugs and won't show
up until later. if both are NULL, the code is correct. if one is NULL, we
should crash immediately because if our list is corrupted, there's no
sensible way of recovering.

Cheers,
   Peter


More information about the wayland-devel mailing list