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 5220c7ba024890adf0d876b9066074c055537e68..6c7f3bdfa0d4f74c93dddc6bf6b2182a1cef0236 100644 (file)
--- 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 882ee6c7d258271c27ab9071225a9edcbf754248..bdefd794d83529f5c1c48a2fe995f6539ac3e715 100644 (file)
--- 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);