[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230504200527.1935944-6-mathieu.desnoyers@efficios.com>
Date: Thu, 4 May 2023 16:05:19 -0400
From: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-kernel@...r.kernel.org,
Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
David Howells <dhowells@...hat.com>,
Ricardo Martinez <ricardo.martinez@...ux.intel.com>
Subject: [RFC PATCH 05/13] list.h: Fix parentheses around macro pointer parameter use
Add missing parentheses around use of macro argument "pos" in those
patterns to ensure operator precedence behaves as expected:
- typeof(*pos)
- pos->member
- "x = y" is changed for "x = (y)", because "y" can be an expression
containing a comma if it is the result of the expansion of a macro such
as #define eval(...) __VA_ARGS__, which would cause unexpected operator
precedence. This use-case is far-fetched, but we have to choose one
way or the other (with or without parentheses) for consistency,
- x && y is changed for (x) && (y).
Remove useless parentheses around use of macro parameter (head) in the
following pattern:
- list_is_head(pos, (head))
Because comma is the lowest priority operator already, so the extra pair
of parentheses is redundant.
This corrects the following usage pattern where operator precedence is
unexpected:
LIST_HEAD(testlist);
struct test {
struct list_head node;
int a;
};
// pos->member issue
void f(void)
{
struct test *t1;
struct test **t2 = &t1;
list_for_each_entry((*t2), &testlist, node) { /* works */
//...
}
list_for_each_entry(*t2, &testlist, node) { /* broken */
//...
}
}
// typeof(*pos) issue
void f2(void)
{
struct test *t1 = NULL, *t2;
t2 = list_prepare_entry((0 + t1), &testlist, node); /* works */
t2 = list_prepare_entry(0 + t1, &testlist, node); /* broken */
}
Note that the macros in which "pos" is also used as an lvalue probably
don't suffer from the lack of parentheses around "pos" in typeof(*pos),
but add those nevertheless to keep everything consistent.
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@...icios.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>
Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc: Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc: David Howells <dhowells@...hat.com>
Cc: Ricardo Martinez <ricardo.martinez@...ux.intel.com>
---
include/linux/list.h | 54 ++++++++++++++++++++++----------------------
1 file changed, 27 insertions(+), 27 deletions(-)
diff --git a/include/linux/list.h b/include/linux/list.h
index f10344dbad4d..d197b1a6f411 100644
--- a/include/linux/list.h
+++ b/include/linux/list.h
@@ -603,7 +603,7 @@ static inline void list_splice_tail_init(struct list_head *list,
* @head: the head for your list.
*/
#define list_for_each(pos, head) \
- for (pos = (head)->next; !list_is_head(pos, (head)); pos = pos->next)
+ for (pos = (head)->next; !list_is_head(pos, head); pos = (pos)->next)
/**
* list_for_each_rcu - Iterate over a list in an RCU-safe fashion
@@ -612,8 +612,8 @@ static inline void list_splice_tail_init(struct list_head *list,
*/
#define list_for_each_rcu(pos, head) \
for (pos = rcu_dereference((head)->next); \
- !list_is_head(pos, (head)); \
- pos = rcu_dereference(pos->next))
+ !list_is_head(pos, head); \
+ pos = rcu_dereference((pos)->next))
/**
* list_for_each_continue - continue iteration over a list
@@ -623,7 +623,7 @@ static inline void list_splice_tail_init(struct list_head *list,
* Continue to iterate over a list, continuing after the current position.
*/
#define list_for_each_continue(pos, head) \
- for (pos = pos->next; !list_is_head(pos, (head)); pos = pos->next)
+ for (pos = (pos)->next; !list_is_head(pos, head); pos = (pos)->next)
/**
* list_for_each_prev - iterate over a list backwards
@@ -631,7 +631,7 @@ static inline void list_splice_tail_init(struct list_head *list,
* @head: the head for your list.
*/
#define list_for_each_prev(pos, head) \
- for (pos = (head)->prev; !list_is_head(pos, (head)); pos = pos->prev)
+ for (pos = (head)->prev; !list_is_head(pos, head); pos = (pos)->prev)
/**
* list_for_each_safe - iterate over a list safe against removal of list entry
@@ -640,9 +640,9 @@ static inline void list_splice_tail_init(struct list_head *list,
* @head: the head for your list.
*/
#define list_for_each_safe(pos, n, head) \
- for (pos = (head)->next, n = pos->next; \
- !list_is_head(pos, (head)); \
- pos = n, n = pos->next)
+ for (pos = (head)->next, n = (pos)->next; \
+ !list_is_head(pos, head); \
+ pos = (n), n = (pos)->next)
/**
* list_for_each_prev_safe - iterate over a list backwards safe against removal of list entry
@@ -651,9 +651,9 @@ static inline void list_splice_tail_init(struct list_head *list,
* @head: the head for your list.
*/
#define list_for_each_prev_safe(pos, n, head) \
- for (pos = (head)->prev, n = pos->prev; \
- !list_is_head(pos, (head)); \
- pos = n, n = pos->prev)
+ for (pos = (head)->prev, n = (pos)->prev; \
+ !list_is_head(pos, head); \
+ pos = (n), n = (pos)->prev)
/**
* list_count_nodes - count nodes in the list
@@ -677,7 +677,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_entry_is_head(pos, head, member) \
- (&pos->member == (head))
+ (&(pos)->member == (head))
/**
* list_for_each_entry - iterate over list of given type
@@ -686,7 +686,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_for_each_entry(pos, head, member) \
- for (pos = list_first_entry(head, typeof(*pos), member); \
+ for (pos = list_first_entry(head, typeof(*(pos)), member); \
!list_entry_is_head(pos, head, member); \
pos = list_next_entry(pos, member))
@@ -697,7 +697,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_for_each_entry_reverse(pos, head, member) \
- for (pos = list_last_entry(head, typeof(*pos), member); \
+ for (pos = list_last_entry(head, typeof(*(pos)), member); \
!list_entry_is_head(pos, head, member); \
pos = list_prev_entry(pos, member))
@@ -710,7 +710,7 @@ static inline size_t list_count_nodes(struct list_head *head)
* Prepares a pos entry for use as a start point in list_for_each_entry_continue().
*/
#define list_prepare_entry(pos, head, member) \
- ((pos) ? : list_entry(head, typeof(*pos), member))
+ ((pos) ? : list_entry(head, typeof(*(pos)), member))
/**
* list_for_each_entry_continue - continue iteration over list of given type
@@ -773,10 +773,10 @@ static inline size_t list_count_nodes(struct list_head *head)
* @member: the name of the list_head within the struct.
*/
#define list_for_each_entry_safe(pos, n, head, member) \
- for (pos = list_first_entry(head, typeof(*pos), member), \
+ for (pos = list_first_entry(head, typeof(*(pos)), member), \
n = list_next_entry(pos, member); \
!list_entry_is_head(pos, head, member); \
- pos = n, n = list_next_entry(n, member))
+ pos = (n), n = list_next_entry(n, member))
/**
* list_for_each_entry_safe_continue - continue list iteration safe against removal
@@ -792,7 +792,7 @@ static inline size_t list_count_nodes(struct list_head *head)
for (pos = list_next_entry(pos, member), \
n = list_next_entry(pos, member); \
!list_entry_is_head(pos, head, member); \
- pos = n, n = list_next_entry(n, member))
+ pos = (n), n = list_next_entry(n, member))
/**
* list_for_each_entry_safe_from - iterate over list from current point safe against removal
@@ -807,7 +807,7 @@ static inline size_t list_count_nodes(struct list_head *head)
#define list_for_each_entry_safe_from(pos, n, head, member) \
for (n = list_next_entry(pos, member); \
!list_entry_is_head(pos, head, member); \
- pos = n, n = list_next_entry(n, member))
+ pos = (n), n = list_next_entry(n, member))
/**
* list_for_each_entry_safe_reverse - iterate backwards over list safe against removal
@@ -820,10 +820,10 @@ static inline size_t list_count_nodes(struct list_head *head)
* of list entry.
*/
#define list_for_each_entry_safe_reverse(pos, n, head, member) \
- for (pos = list_last_entry(head, typeof(*pos), member), \
+ for (pos = list_last_entry(head, typeof(*(pos)), member), \
n = list_prev_entry(pos, member); \
!list_entry_is_head(pos, head, member); \
- pos = n, n = list_prev_entry(n, member))
+ pos = (n), n = list_prev_entry(n, member))
/**
* list_safe_reset_next - reset a stale list_for_each_entry_safe loop
@@ -1033,11 +1033,11 @@ static inline void hlist_move_list(struct hlist_head *old,
#define hlist_entry(ptr, type, member) container_of(ptr,type,member)
#define hlist_for_each(pos, head) \
- for (pos = (head)->first; pos ; pos = pos->next)
+ for (pos = (head)->first; pos ; pos = (pos)->next)
#define hlist_for_each_safe(pos, n, head) \
- for (pos = (head)->first; pos && ({ n = pos->next; 1; }); \
- pos = n)
+ for (pos = (head)->first; (pos) && ({ n = (pos)->next; 1; }); \
+ pos = (n))
#define hlist_entry_safe(ptr, type, member) \
({ typeof(ptr) ____ptr = (ptr); \
@@ -1082,8 +1082,8 @@ static inline void hlist_move_list(struct hlist_head *old,
* @member: the name of the hlist_node within the struct.
*/
#define hlist_for_each_entry_safe(pos, n, head, member) \
- for (pos = hlist_entry_safe((head)->first, typeof(*pos), member);\
- pos && ({ n = pos->member.next; 1; }); \
- pos = hlist_entry_safe(n, typeof(*pos), member))
+ for (pos = hlist_entry_safe((head)->first, typeof(*(pos)), member);\
+ (pos) && ({ n = (pos)->member.next; 1; }); \
+ pos = hlist_entry_safe(n, typeof(*(pos)), member))
#endif
--
2.25.1
Powered by blists - more mailing lists