From: Peter Hutterer <peter.hutterer@who-t.net>
Date: Thu, 22 Jul 2021 01:37:27 +0000 (+1000)
Subject: util-list: restore list_for_each_safe() to be a single statement
X-Git-Tag: 1.18.901~40
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=492f6817d37d05064d50e93d3abe4600768ab3d3;p=platform%2Fupstream%2Flibinput.git

util-list: restore list_for_each_safe() to be a single statement

3d3d9b7f69b1119523becab3160378066db2e1c0 got rid of the need for a tmp
argument for list_for_each_safe() but switched the loop to be a
multiline statement. This could potentially cause bugs where the loop is
used inside a block without curly braces, e.g.

    if (condition)
        list_for_each_safe()
            func()

The assignment preceding the actual loop would result in the code
reading as:

    if (condition)
        pos = ....

    list_for_each_safe()

The actual list loop would be unconditional.

Fix this by moving the initial assignment into an expression statement.

Signed-off-by: Peter Hutterer <peter.hutterer@who-t.net>
---

diff --git a/src/util-list.h b/src/util-list.h
index 5220c7ba..6c7f3bdf 100644
--- a/src/util-list.h
+++ b/src/util-list.h
@@ -189,8 +189,10 @@ bool list_empty(const struct list *list);
  * @endcode
  */
 #define list_for_each_safe(pos, head, member)				\
-	pos = list_first_entry_by_type(head, __typeof__(*pos), member);			\
-	for (__typeof__(pos) _tmp = list_first_entry_by_type(&pos->member, __typeof__(*_tmp), member); \
+	for (__typeof__(pos) _tmp = ({ \
+				     pos = list_first_entry_by_type(head, __typeof__(*pos), member);	\
+				     list_first_entry_by_type(&pos->member, __typeof__(*_tmp), member); \
+				     }); \
 	     &pos->member != (head);					\
 	     pos = _tmp,						\
 	     _tmp = list_first_entry_by_type(&pos->member, __typeof__(*_tmp), member))
diff --git a/test/test-utils.c b/test/test-utils.c
index 882ee6c7..bdefd794 100644
--- a/test/test-utils.c
+++ b/test/test-utils.c
@@ -1295,6 +1295,39 @@ START_TEST(list_test_append)
 }
 END_TEST
 
+START_TEST(list_test_foreach)
+{
+	struct list_test {
+		int val;
+		struct list node;
+	} tests[] = {
+		{ .val  = 1 },
+		{ .val  = 2 },
+		{ .val  = 3 },
+		{ .val  = 4 },
+	};
+	struct list_test *t;
+	struct list head;
+
+	list_init(&head);
+
+	ARRAY_FOR_EACH(tests, t) {
+		list_append(&head, &t->node);
+	}
+
+	/* Make sure both loop macros are a single line statement */
+	if (false)
+		list_for_each(t, &head, node) {
+			ck_abort_msg("We should not get here");
+		}
+
+	if (false)
+		list_for_each_safe(t, &head, node) {
+			ck_abort_msg("We should not get here");
+		}
+}
+END_TEST
+
 START_TEST(strverscmp_test)
 {
 	ck_assert_int_eq(libinput_strverscmp("", ""), 0);
@@ -1427,6 +1460,7 @@ litest_utils_suite(void)
 
 	tcase_add_test(tc, list_test_insert);
 	tcase_add_test(tc, list_test_append);
+	tcase_add_test(tc, list_test_foreach);
 	tcase_add_test(tc, strverscmp_test);
 	tcase_add_test(tc, streq_test);
 	tcase_add_test(tc, strneq_test);