Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1664853 > unrolled thread

[PATCH v2 0/2] Avoid namespace collision within macros & tidyup

Started byRamesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
First post2017-06-13 15:50 +0200
Last post2017-06-14 09:30 +0200
Articles 7 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH v2 0/2] Avoid namespace collision within macros & tidyup Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> - 2017-06-13 15:50 +0200
    [PATCH v2 2/2] regmap: Avoid namespace collision within macro & tidyup Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> - 2017-06-13 15:50 +0200
    [PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> - 2017-06-13 15:50 +0200
      Re: [PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup Geert Uytterhoeven <geert@linux-m68k.org> - 2017-06-13 16:20 +0200
    Re: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup Ian Arkver <ian.arkver.dev@gmail.com> - 2017-06-14 08:50 +0200
      RE: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com> - 2017-06-14 09:20 +0200
        Re: [PATCH v2 0/2] Avoid namespace collision within macros & tidyup Ian Arkver <ian.arkver.dev@gmail.com> - 2017-06-14 09:30 +0200

#1664853 — [PATCH v2 0/2] Avoid namespace collision within macros & tidyup

FromRamesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Date2017-06-13 15:50 +0200
Subject[PATCH v2 0/2] Avoid namespace collision within macros & tidyup
Message-ID<tRUmJ-ap-1@gated-at.bofh.it>
Hi All,

The readx_poll_timeout & similar macros defines local variable that can
cause name space collision with the caller. Fixed this issue by prefixing
them with underscores. Also tidied couple of instances where the macro
arguments are used in expressions without paranthesis.

This patchset is based on top of today's linux-next repo.
commit bc4c75f41a1c ("Add linux-next specific files for 20170613")

Change history:

v2:
 - iopoll.h:
	- Enclosed timeout_us & sleep_us arguments with paranthesis
 - regmap.h:
	- Enclosed timeout_us & sleep_us arguments with paranthesis
	- Renamed pollret to __ret

Note: timeout_us cause spare check warning as identified here [1].

[1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg15138.html

Thanks,
Ramesh

Ramesh Shanmugasundaram (2):
  iopoll: Avoid namespace collision within macros & tidyup
  regmap: Avoid namespace collision within macro & tidyup

 include/linux/iopoll.h | 12 +++++++-----
 include/linux/regmap.h | 17 +++++++++--------
 2 files changed, 16 insertions(+), 13 deletions(-)

-- 
2.12.2

[toc] | [next] | [standalone]

#1664866 — [PATCH v2 2/2] regmap: Avoid namespace collision within macro & tidyup

FromRamesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Date2017-06-13 15:50 +0200
Subject[PATCH v2 2/2] regmap: Avoid namespace collision within macro & tidyup
Message-ID<tRUmK-ap-35@gated-at.bofh.it>
In reply to#1664853
Renamed variable "timeout" to "__timeout" & "pollret" to "__ret" to
avoid namespace collision. Tidy up macro arguments with paranthesis.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 include/linux/regmap.h | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/include/linux/regmap.h b/include/linux/regmap.h
index 86eeacc1425a..ebc7282abc80 100644
--- a/include/linux/regmap.h
+++ b/include/linux/regmap.h
@@ -120,23 +120,24 @@ struct reg_sequence {
  */
 #define regmap_read_poll_timeout(map, addr, val, cond, sleep_us, timeout_us) \
 ({ \
-	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
-	int pollret; \
+	ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \
+	int __ret; \
 	might_sleep_if(sleep_us); \
 	for (;;) { \
-		pollret = regmap_read((map), (addr), &(val)); \
-		if (pollret) \
+		__ret = regmap_read((map), (addr), &(val)); \
+		if (__ret) \
 			break; \
 		if (cond) \
 			break; \
-		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
-			pollret = regmap_read((map), (addr), &(val)); \
+		if ((timeout_us) && \
+		    ktime_compare(ktime_get(), __timeout) > 0) { \
+			__ret = regmap_read((map), (addr), &(val)); \
 			break; \
 		} \
 		if (sleep_us) \
-			usleep_range((sleep_us >> 2) + 1, sleep_us); \
+			usleep_range(((sleep_us) >> 2) + 1, sleep_us); \
 	} \
-	pollret ?: ((cond) ? 0 : -ETIMEDOUT); \
+	__ret ?: ((cond) ? 0 : -ETIMEDOUT); \
 })
 
 #ifdef CONFIG_REGMAP
-- 
2.12.2

[toc] | [prev] | [next] | [standalone]

#1664868 — [PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup

FromRamesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Date2017-06-13 15:50 +0200
Subject[PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup
Message-ID<tRUmK-ap-39@gated-at.bofh.it>
In reply to#1664853
Renamed variable "timeout" to "__timeout" to avoid namespace collision.
Tidy up macro arguments with paranthesis.

Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
---
 include/linux/iopoll.h | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/include/linux/iopoll.h b/include/linux/iopoll.h
index d29e1e21bf3f..e000172bee54 100644
--- a/include/linux/iopoll.h
+++ b/include/linux/iopoll.h
@@ -42,18 +42,19 @@
  */
 #define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)	\
 ({ \
-	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \
 	might_sleep_if(sleep_us); \
 	for (;;) { \
 		(val) = op(addr); \
 		if (cond) \
 			break; \
-		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+		if ((timeout_us) && \
+		    ktime_compare(ktime_get(), __timeout) > 0) { \
 			(val) = op(addr); \
 			break; \
 		} \
 		if (sleep_us) \
-			usleep_range((sleep_us >> 2) + 1, sleep_us); \
+			usleep_range(((sleep_us) >> 2) + 1, sleep_us); \
 	} \
 	(cond) ? 0 : -ETIMEDOUT; \
 })
@@ -77,12 +78,13 @@
  */
 #define readx_poll_timeout_atomic(op, addr, val, cond, delay_us, timeout_us) \
 ({ \
-	ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
+	ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \
 	for (;;) { \
 		(val) = op(addr); \
 		if (cond) \
 			break; \
-		if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
+		if ((timeout_us) && \
+		    ktime_compare(ktime_get(), __timeout) > 0) { \
 			(val) = op(addr); \
 			break; \
 		} \
-- 
2.12.2

[toc] | [prev] | [next] | [standalone]

#1664921 — Re: [PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup

FromGeert Uytterhoeven <geert@linux-m68k.org>
Date2017-06-13 16:20 +0200
SubjectRe: [PATCH v2 1/2] iopoll: Avoid namespace collision within macros & tidyup
Message-ID<tRUPM-zh-17@gated-at.bofh.it>
In reply to#1664868
Hi Ramesh,

On Tue, Jun 13, 2017 at 3:33 PM, Ramesh Shanmugasundaram
<ramesh.shanmugasundaram@bp.renesas.com> wrote:
> Renamed variable "timeout" to "__timeout" to avoid namespace collision.
> Tidy up macro arguments with paranthesis.
>
> Signed-off-by: Ramesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>

Thanks for your patches!

> --- a/include/linux/iopoll.h
> +++ b/include/linux/iopoll.h
> @@ -42,18 +42,19 @@
>   */
>  #define readx_poll_timeout(op, addr, val, cond, sleep_us, timeout_us)  \
>  ({ \
> -       ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
> +       ktime_t __timeout = ktime_add_us(ktime_get(), timeout_us); \

I think timeout_us should be within parentheses, too.

>         might_sleep_if(sleep_us); \
>         for (;;) { \
>                 (val) = op(addr); \
>                 if (cond) \
>                         break; \
> -               if (timeout_us && ktime_compare(ktime_get(), timeout) > 0) { \
> +               if ((timeout_us) && \
> +                   ktime_compare(ktime_get(), __timeout) > 0) { \
>                         (val) = op(addr); \
>                         break; \
>                 } \
>                 if (sleep_us) \
> -                       usleep_range((sleep_us >> 2) + 1, sleep_us); \
> +                       usleep_range(((sleep_us) >> 2) + 1, sleep_us); \

Same for sleep_us.

Also in readx_poll_timeout_atomic(), and in your second patch.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

[toc] | [prev] | [next] | [standalone]

#1665486

FromIan Arkver <ian.arkver.dev@gmail.com>
Date2017-06-14 08:50 +0200
Message-ID<tSahQ-1IN-9@gated-at.bofh.it>
In reply to#1664853
On 13/06/17 14:33, Ramesh Shanmugasundaram wrote:
> Hi All,
> 
> The readx_poll_timeout & similar macros defines local variable that can
> cause name space collision with the caller. Fixed this issue by prefixing
> them with underscores.

The compound statement has a local variable scope, so these won't 
collide with the caller I believe.

> Also tidied couple of instances where the macro
> arguments are used in expressions without paranthesis.
> 
> This patchset is based on top of today's linux-next repo.
> commit bc4c75f41a1c ("Add linux-next specific files for 20170613")
> 
> Change history:
> 
> v2:
>   - iopoll.h:
> 	- Enclosed timeout_us & sleep_us arguments with paranthesis
>   - regmap.h:
> 	- Enclosed timeout_us & sleep_us arguments with paranthesis
> 	- Renamed pollret to __ret
> 
> Note: timeout_us cause spare check warning as identified here [1].
> 
> [1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg15138.html
> 
> Thanks,
> Ramesh
> 
> Ramesh Shanmugasundaram (2):
>    iopoll: Avoid namespace collision within macros & tidyup
>    regmap: Avoid namespace collision within macro & tidyup
> 
>   include/linux/iopoll.h | 12 +++++++-----
>   include/linux/regmap.h | 17 +++++++++--------
>   2 files changed, 16 insertions(+), 13 deletions(-)
>

[toc] | [prev] | [next] | [standalone]

#1665499

FromRamesh Shanmugasundaram <ramesh.shanmugasundaram@bp.renesas.com>
Date2017-06-14 09:20 +0200
Message-ID<tSaKS-2aJ-11@gated-at.bofh.it>
In reply to#1665486
> Subject: Re: [PATCH v2 0/2] Avoid namespace collision within macros &
> tidyup
> 
> On 13/06/17 14:33, Ramesh Shanmugasundaram wrote:
> > Hi All,
> >
> > The readx_poll_timeout & similar macros defines local variable that
> > can cause name space collision with the caller. Fixed this issue by
> > prefixing them with underscores.
> 
> The compound statement has a local variable scope, so these won't collide
> with the caller I believe.

But xxx_poll_timeout is a macro??

Usage regmap_read_poll_timeout(..., timeout) with variable name "timeout" in the caller results in 

include/linux/regmap.h:123:20: warning: 'timeout' is used uninitialized in this function [-Wuninitialized]
  ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \

> 
> > Also tidied couple of instances where the macro arguments are used in
> > expressions without paranthesis.
> >
> > This patchset is based on top of today's linux-next repo.
> > commit bc4c75f41a1c ("Add linux-next specific files for 20170613")
> >
> > Change history:
> >
> > v2:
> >   - iopoll.h:
> > 	- Enclosed timeout_us & sleep_us arguments with paranthesis
> >   - regmap.h:
> > 	- Enclosed timeout_us & sleep_us arguments with paranthesis
> > 	- Renamed pollret to __ret
> >
> > Note: timeout_us cause spare check warning as identified here [1].
> >
> > [1]
> > https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg1513
> > 8.html
> >
> > Thanks,
> > Ramesh
> >
> > Ramesh Shanmugasundaram (2):
> >    iopoll: Avoid namespace collision within macros & tidyup
> >    regmap: Avoid namespace collision within macro & tidyup
> >
> >   include/linux/iopoll.h | 12 +++++++-----
> >   include/linux/regmap.h | 17 +++++++++--------
> >   2 files changed, 16 insertions(+), 13 deletions(-)
> >

[toc] | [prev] | [next] | [standalone]

#1665513

FromIan Arkver <ian.arkver.dev@gmail.com>
Date2017-06-14 09:30 +0200
Message-ID<tSaUy-2dU-33@gated-at.bofh.it>
In reply to#1665499
On 14/06/17 08:18, Ramesh Shanmugasundaram wrote:
>> Subject: Re: [PATCH v2 0/2] Avoid namespace collision within macros &
>> tidyup
>>
>> On 13/06/17 14:33, Ramesh Shanmugasundaram wrote:
>>> Hi All,
>>>
>>> The readx_poll_timeout & similar macros defines local variable that
>>> can cause name space collision with the caller. Fixed this issue by
>>> prefixing them with underscores.
>>
>> The compound statement has a local variable scope, so these won't collide
>> with the caller I believe.
> 
> But xxx_poll_timeout is a macro??
> 
> Usage regmap_read_poll_timeout(..., timeout) with variable name "timeout" in the caller results in
> 
> include/linux/regmap.h:123:20: warning: 'timeout' is used uninitialized in this function [-Wuninitialized]
>    ktime_t timeout = ktime_add_us(ktime_get(), timeout_us); \
> 

Oh right, collide with a passed in variable, yes. Sorry.

>>
>>> Also tidied couple of instances where the macro arguments are used in
>>> expressions without paranthesis.
>>>
>>> This patchset is based on top of today's linux-next repo.
>>> commit bc4c75f41a1c ("Add linux-next specific files for 20170613")
>>>
>>> Change history:
>>>
>>> v2:
>>>    - iopoll.h:
>>> 	- Enclosed timeout_us & sleep_us arguments with paranthesis
>>>    - regmap.h:
>>> 	- Enclosed timeout_us & sleep_us arguments with paranthesis
>>> 	- Renamed pollret to __ret
>>>
>>> Note: timeout_us cause spare check warning as identified here [1].
>>>
>>> [1]
>>> https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg1513
>>> 8.html
>>>
>>> Thanks,
>>> Ramesh
>>>
>>> Ramesh Shanmugasundaram (2):
>>>     iopoll: Avoid namespace collision within macros & tidyup
>>>     regmap: Avoid namespace collision within macro & tidyup
>>>
>>>    include/linux/iopoll.h | 12 +++++++-----
>>>    include/linux/regmap.h | 17 +++++++++--------
>>>    2 files changed, 16 insertions(+), 13 deletions(-)
>>>

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web