list.h: don't crash when removing an element from a NULL list

Submitted by Peter Hutterer on July 5, 2012, 6:17 a.m.

Details

Message ID 20120705061756.GA6806@yabbi.bne.redhat.com
State Accepted
Commit 167993254a5cbe11a1f44fad1e8ae042089c1619
Headers show

Not browsing as part of any series.

Commit Message

Peter Hutterer July 5, 2012, 6:17 a.m.
Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---
Keith, please merge this directly if you're happy with it.

 include/list.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Patch hide | download patch | download mbox

diff --git a/include/list.h b/include/list.h
index 96c0bcb..d54a207 100644
--- a/include/list.h
+++ b/include/list.h
@@ -453,7 +453,7 @@  xorg_list_is_empty(struct xorg_list *head)
 #define nt_list_del(_entry, _list, _type, _member)		\
 	do {							\
 		_type *__e = _entry;				\
-		if (__e == NULL) break;				\
+		if (__e == NULL || _list == NULL) break;        \
 		if ((_list) == __e) {				\
 		    _list = __e->_member;			\
 		} else {					\

Comments

Peter Hutterer <peter.hutterer@who-t.net> writes:

> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> Keith, please merge this directly if you're happy with it.

It looks fine, but I don't see any place that this actually matters in
current code?

(the more I see of these twisty list macros, the more I prefer
open-coded lists though; wow this is hard to understand).
On Wed, Jul 04, 2012 at 11:32:38PM -0700, Keith Packard wrote:
> Peter Hutterer <peter.hutterer@who-t.net> writes:
> 
> > Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> > ---
> > Keith, please merge this directly if you're happy with it.
> 
> It looks fine, but I don't see any place that this actually matters in
> current code?

no, but the selinux fix will need it (partially out of laziness, partially
to make the code nicer).

> (the more I see of these twisty list macros, the more I prefer
> open-coded lists though; wow this is hard to understand).

given how often we've found bugs in open-coded lists I disagree.
yes, they're hairy but they're tested, the macro behaves the same
everywhere. and if you really worry about the code being wrong, run it
through the pre-processor and it will look like an open-coded list.

Cheers,
  Peter
Peter Hutterer <peter.hutterer@who-t.net> writes:

> no, but the selinux fix will need it (partially out of laziness, partially
> to make the code nicer).

Ok. I'll merged it in then.

>> (the more I see of these twisty list macros, the more I prefer
>> open-coded lists though; wow this is hard to understand).
>
> given how often we've found bugs in open-coded lists I disagree.
> yes, they're hairy but they're tested, the macro behaves the same
> everywhere. and if you really worry about the code being wrong, run it
> through the pre-processor and it will look like an open-coded list.

It's all a tradeoff -- I just don't like having to learn a new
programming language each time I see a new set of list macros (or other
macro programming of this nature). Reminds me of how loathsome C++
templates are.
Peter Hutterer <peter.hutterer@who-t.net> writes:

> Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
> ---
> Keith, please merge this directly if you're happy with it.

Merged.
   8aa6d49..1679932  master -> master