Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1739572 > unrolled thread
| Started by | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| First post | 2017-09-26 09:00 +0200 |
| Last post | 2017-09-26 10:40 +0200 |
| Articles | 5 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() Byungchul Park <byungchul.park@lge.com> - 2017-09-26 09:00 +0200
Re: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() "Huang\, Ying" <ying.huang@intel.com> - 2017-09-26 09:10 +0200
RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) <byungchul.park@lge.com> - 2017-09-26 10:10 +0200
Re: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() "Huang\, Ying" <ying.huang@intel.com> - 2017-09-26 10:20 +0200
RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) <byungchul.park@lge.com> - 2017-09-26 10:40 +0200
| From | Byungchul Park <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-26 09:00 +0200 |
| Subject | [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() |
| Message-ID | <utS0y-18O-23@gated-at.bofh.it> |
It would be somewhat safer to put parentheses around parameters of a macro with parameters. Put it. Signed-off-by: Byungchul Park <byungchul.park@lge.com> --- include/linux/llist.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/include/linux/llist.h b/include/linux/llist.h index 1957635..e280b297 100644 --- a/include/linux/llist.h +++ b/include/linux/llist.h @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head *list) * reverse the order by yourself before traversing. */ #define llist_for_each_entry_safe(pos, n, node, member) \ - for (pos = llist_entry((node), typeof(*pos), member); \ + for ((pos) = llist_entry((node), typeof(*(pos)), member); \ member_address_is_nonnull(pos, member) && \ - (n = llist_entry(pos->member.next, typeof(*n), member), true); \ - pos = n) + ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); \ + (pos) = (n)) /** * llist_empty - tests whether a lock-less list is empty -- 1.9.1
[toc] | [next] | [standalone]
| From | "Huang\, Ying" <ying.huang@intel.com> |
|---|---|
| Date | 2017-09-26 09:10 +0200 |
| Message-ID | <utSae-1rM-13@gated-at.bofh.it> |
| In reply to | #1739572 |
Hi, Byungchul, Byungchul Park <byungchul.park@lge.com> writes: > It would be somewhat safer to put parentheses around parameters of > a macro with parameters. Put it. > > Signed-off-by: Byungchul Park <byungchul.park@lge.com> > --- > include/linux/llist.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/linux/llist.h b/include/linux/llist.h > index 1957635..e280b297 100644 > --- a/include/linux/llist.h > +++ b/include/linux/llist.h > @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head *list) > * reverse the order by yourself before traversing. > */ > #define llist_for_each_entry_safe(pos, n, node, member) \ > - for (pos = llist_entry((node), typeof(*pos), member); \ > + for ((pos) = llist_entry((node), typeof(*(pos)), member); \ > member_address_is_nonnull(pos, member) && \ > - (n = llist_entry(pos->member.next, typeof(*n), member), true); \ > - pos = n) > + ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); \ > + (pos) = (n)) > > /** > * llist_empty - tests whether a lock-less list is empty The original code follows the style of list_for_each_entry_safe(). The parameters "pos" and "n" must be variable. Because list_xxx family functions work well so far, I think we needn't to change it too. Best Regards, Huang, Ying
[toc] | [prev] | [next] | [standalone]
| From | 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-26 10:10 +0200 |
| Subject | RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() |
| Message-ID | <utT6j-26b-31@gated-at.bofh.it> |
| In reply to | #1739583 |
> -----Original Message----- > From: Huang, Ying [mailto:ying.huang@intel.com] > Sent: Tuesday, September 26, 2017 4:02 PM > To: Byungchul Park > Cc: peterz@infradead.org; mingo@kernel.org; linux-kernel@vger.kernel.org; > kernel-team@lge.com; ying.huang@intel.com > Subject: Re: [PATCH] llist: Put parentheses around parameters of > llist_for_each_entry_safe() > > Hi, Byungchul, > > Byungchul Park <byungchul.park@lge.com> writes: > > > It would be somewhat safer to put parentheses around parameters of > > a macro with parameters. Put it. > > > > Signed-off-by: Byungchul Park <byungchul.park@lge.com> > > --- > > include/linux/llist.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/linux/llist.h b/include/linux/llist.h > > index 1957635..e280b297 100644 > > --- a/include/linux/llist.h > > +++ b/include/linux/llist.h > > @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head *list) > > * reverse the order by yourself before traversing. > > */ > > #define llist_for_each_entry_safe(pos, n, node, member) > \ > > - for (pos = llist_entry((node), typeof(*pos), member); \ > > + for ((pos) = llist_entry((node), typeof(*(pos)), member); \ > > member_address_is_nonnull(pos, member) && > \ > > - (n = llist_entry(pos->member.next, typeof(*n), member), true); \ > > - pos = n) > > + ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); > \ > > + (pos) = (n)) > > > > /** > > * llist_empty - tests whether a lock-less list is empty > > The original code follows the style of list_for_each_entry_safe(). The Hello Huang, I don’t see what you say here exactly, but let me note that all llist macros are safe except the llist_for_each_entry_safe(). > parameters "pos" and "n" must be variable. Because list_xxx family > functions work well so far, I think we needn't to change it too. I see. I don't want to argue much wrt such a trivial thing but I think it would be better to fix it since the fix is fairly simple and clear. However, it's ok if the fix introduces a bad thing at least. > Best Regards, > Huang, Ying
[toc] | [prev] | [next] | [standalone]
| From | "Huang\, Ying" <ying.huang@intel.com> |
|---|---|
| Date | 2017-09-26 10:20 +0200 |
| Message-ID | <utTfY-29X-17@gated-at.bofh.it> |
| In reply to | #1739658 |
"박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)" <byungchul.park@lge.com> writes: >> -----Original Message----- >> From: Huang, Ying [mailto:ying.huang@intel.com] >> Sent: Tuesday, September 26, 2017 4:02 PM >> To: Byungchul Park >> Cc: peterz@infradead.org; mingo@kernel.org; linux-kernel@vger.kernel.org; >> kernel-team@lge.com; ying.huang@intel.com >> Subject: Re: [PATCH] llist: Put parentheses around parameters of >> llist_for_each_entry_safe() >> >> Hi, Byungchul, >> >> Byungchul Park <byungchul.park@lge.com> writes: >> >> > It would be somewhat safer to put parentheses around parameters of >> > a macro with parameters. Put it. >> > >> > Signed-off-by: Byungchul Park <byungchul.park@lge.com> >> > --- >> > include/linux/llist.h | 6 +++--- >> > 1 file changed, 3 insertions(+), 3 deletions(-) >> > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h >> > index 1957635..e280b297 100644 >> > --- a/include/linux/llist.h >> > +++ b/include/linux/llist.h >> > @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head *list) >> > * reverse the order by yourself before traversing. >> > */ >> > #define llist_for_each_entry_safe(pos, n, node, member) >> \ >> > - for (pos = llist_entry((node), typeof(*pos), member); \ >> > + for ((pos) = llist_entry((node), typeof(*(pos)), member); \ >> > member_address_is_nonnull(pos, member) && >> \ >> > - (n = llist_entry(pos->member.next, typeof(*n), member), true); \ >> > - pos = n) >> > + ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); >> \ >> > + (pos) = (n)) >> > >> > /** >> > * llist_empty - tests whether a lock-less list is empty >> >> The original code follows the style of list_for_each_entry_safe(). The > > Hello Huang, > > I don’t see what you say here exactly, but let me note that all llist macros > are safe except the llist_for_each_entry_safe(). > >> parameters "pos" and "n" must be variable. Because list_xxx family >> functions work well so far, I think we needn't to change it too. > > I see. I don't want to argue much wrt such a trivial thing but I think > it would be better to fix it since the fix is fairly simple and clear. > However, it's ok if the fix introduces a bad thing at least. Yes, it's simple. But I don't think it helps too. Considering that list family functions with same style have no issues. Best Regards, Huang, Ying
[toc] | [prev] | [next] | [standalone]
| From | 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) <byungchul.park@lge.com> |
|---|---|
| Date | 2017-09-26 10:40 +0200 |
| Subject | RE: [PATCH] llist: Put parentheses around parameters of llist_for_each_entry_safe() |
| Message-ID | <utTzk-2jm-21@gated-at.bofh.it> |
| In reply to | #1739661 |
> -----Original Message----- > From: Huang, Ying [mailto:ying.huang@intel.com] > Sent: Tuesday, September 26, 2017 5:14 PM > To: 박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com) > Cc: Huang, Ying; peterz@infradead.org; mingo@kernel.org; linux- > kernel@vger.kernel.org; kernel-team@lge.com > Subject: Re: [PATCH] llist: Put parentheses around parameters of > llist_for_each_entry_safe() > > "박병철/선임연구원/SW Platform(연)AOT팀(byungchul.park@lge.com)" > <byungchul.park@lge.com> writes: > > >> -----Original Message----- > >> From: Huang, Ying [mailto:ying.huang@intel.com] > >> Sent: Tuesday, September 26, 2017 4:02 PM > >> To: Byungchul Park > >> Cc: peterz@infradead.org; mingo@kernel.org; linux-kernel@vger.kernel.org; > >> kernel-team@lge.com; ying.huang@intel.com > >> Subject: Re: [PATCH] llist: Put parentheses around parameters of > >> llist_for_each_entry_safe() > >> > >> Hi, Byungchul, > >> > >> Byungchul Park <byungchul.park@lge.com> writes: > >> > >> > It would be somewhat safer to put parentheses around parameters of > >> > a macro with parameters. Put it. > >> > > >> > Signed-off-by: Byungchul Park <byungchul.park@lge.com> > >> > --- > >> > include/linux/llist.h | 6 +++--- > >> > 1 file changed, 3 insertions(+), 3 deletions(-) > >> > > >> > diff --git a/include/linux/llist.h b/include/linux/llist.h > >> > index 1957635..e280b297 100644 > >> > --- a/include/linux/llist.h > >> > +++ b/include/linux/llist.h > >> > @@ -183,10 +183,10 @@ static inline void init_llist_head(struct llist_head > *list) > >> > * reverse the order by yourself before traversing. > >> > */ > >> > #define llist_for_each_entry_safe(pos, n, node, member) > >> \ > >> > - for (pos = llist_entry((node), typeof(*pos), member); \ > >> > + for ((pos) = llist_entry((node), typeof(*(pos)), member); \ > >> > member_address_is_nonnull(pos, member) && > >> \ > >> > - (n = llist_entry(pos->member.next, typeof(*n), member), true); \ > >> > - pos = n) > >> > + ((n) = llist_entry((pos)->member.next, typeof(*(n)), member), true); > >> \ > >> > + (pos) = (n)) > >> > > >> > /** > >> > * llist_empty - tests whether a lock-less list is empty > >> > >> The original code follows the style of list_for_each_entry_safe(). The > > > > Hello Huang, > > > > I don’t see what you say here exactly, but let me note that all llist macros > > are safe except the llist_for_each_entry_safe(). > > > >> parameters "pos" and "n" must be variable. Because list_xxx family > >> functions work well so far, I think we needn't to change it too. > > > > I see. I don't want to argue much wrt such a trivial thing but I think > > it would be better to fix it since the fix is fairly simple and clear. > > However, it's ok if the fix introduces a bad thing at least. > > Yes, it's simple. But I don't think it helps too. Considering that > list family functions with same style have no issues. In addition, making all the llist macros implementation consistent is also meaningful, isn't it? Ok. I think your opinion is right and mine is also meaningful. I will just follow the majority opinion. Thank you for giving an opinion.
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web