dovecot-2.2: lib: DLLIST*_REMOVE*() no longer breaks the linked ...

[dovecot at dovecot.org]

details:   http://hg.dovecot.org/dovecot-2.2/rev/46e47e4082d8
changeset: 17577:46e47e4082d8
user:      Timo Sirainen <tss at iki.fi>
date:      Thu Jul 03 16:07:09 2014 +0300
description:
lib: DLLIST*_REMOVE*() no longer breaks the linked list if we try to remove item that doesn't exist there.
Hopefully there wasn't any code that actually did this, but it's safer this
way anyway. Perhaps it could be even made to assert-crash if it happens.

diffstat:

 src/lib/llist.h      |  19 +++++++++----------
 src/lib/test-llist.c |  13 +++++++++++++
 2 files changed, 22 insertions(+), 10 deletions(-)

diffs (84 lines):

diff -r af665736536c -r 46e47e4082d8 src/lib/llist.h
--- a/src/lib/llist.h	Thu Jul 03 15:26:32 2014 +0300
+++ b/src/lib/llist.h	Thu Jul 03 16:07:09 2014 +0300
@@ -13,10 +13,10 @@
 	DLLIST_PREPEND_FULL(list, item, prev, next)
 
 #define DLLIST_REMOVE_FULL(list, item, prev, next) STMT_START { \
-	if ((item)->prev == NULL) \
+	if ((item)->prev != NULL) \
+		(item)->prev->next = (item)->next; \
+	else if ((*list) == item) \
 		*(list) = (item)->next; \
-	else \
-		(item)->prev->next = (item)->next; \
 	if ((item)->next != NULL) { \
 		(item)->next->prev = (item)->prev; \
 		(item)->next = NULL; \
@@ -61,16 +61,15 @@
 	DLLIST2_INSERT_AFTER_FULL(head, tail, after, item, prev, next)
 
 #define DLLIST2_REMOVE_FULL(head, tail, item, prev, next) STMT_START { \
-	if ((item)->prev == NULL) \
+	if ((item)->prev != NULL) \
+		(item)->prev->next = (item)->next; \
+	else if (*(head) == item) \
 		*(head) = (item)->next; \
-	else \
-		(item)->prev->next = (item)->next; \
-	if ((item)->next == NULL) \
-		*(tail) = (item)->prev; \
-	else { \
+	if ((item)->next != NULL) { \
 		(item)->next->prev = (item)->prev; \
 		(item)->next = NULL; \
-	} \
+	} else if ((*tail) == item) \
+		*(tail) = (item)->prev; \
 	(item)->prev = NULL; \
 	} STMT_END
 
diff -r af665736536c -r 46e47e4082d8 src/lib/test-llist.c
--- a/src/lib/test-llist.c	Thu Jul 03 15:26:32 2014 +0300
+++ b/src/lib/test-llist.c	Thu Jul 03 16:07:09 2014 +0300
@@ -12,6 +12,7 @@
 static void test_dllist(void)
 {
 	struct dllist *head = NULL, *l4, *l3, *l2, *l1;
+	struct dllist empty = { NULL, NULL };
 
 	l4 = t_new(struct dllist, 1);
 	l3 = t_new(struct dllist, 1);
@@ -48,6 +49,11 @@
 	test_assert(head == l1);
 	test_assert(l1->prev == NULL && l1->next == l3);
 	test_assert(l3->prev == l1 && l3->next == NULL);
+	/* removal of an entry not in the list shouldn't cause the list to break */
+	DLLIST_REMOVE(&head, &empty);
+	test_assert(head == l1);
+	test_assert(l1->prev == NULL && l1->next == l3);
+	test_assert(l3->prev == l1 && l3->next == NULL);
 	/* remove last two */
 	DLLIST_REMOVE(&head, l1);
 	DLLIST_REMOVE(&head, l3);
@@ -59,6 +65,7 @@
 static void test_dllist2(void)
 {
 	struct dllist *head = NULL, *tail = NULL, *l4, *l3, *l2, *l1;
+	struct dllist empty = { NULL, NULL };
 
 	l4 = t_new(struct dllist, 1);
 	l3 = t_new(struct dllist, 1);
@@ -111,6 +118,12 @@
 	test_assert(head == l1 && tail == l3);
 	test_assert(l1->prev == NULL && l1->next == l3);
 	test_assert(l3->prev == l1 && l3->next == NULL);
+	/* removal of an entry not in the list shouldn't cause the list to break */
+	DLLIST2_REMOVE(&head, &tail, &empty);
+	test_assert(head == l1);
+	test_assert(head == l1 && tail == l3);
+	test_assert(l1->prev == NULL && l1->next == l3);
+	test_assert(l3->prev == l1 && l3->next == NULL);
 	/* remove last two */
 	DLLIST2_REMOVE(&head, &tail, l1);
 	DLLIST2_REMOVE(&head, &tail, l3);