[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Yh5tdcNNHw/z7VRZ@casper.infradead.org>
Date: Tue, 1 Mar 2022 19:01:09 +0000
From: Matthew Wilcox <willy@...radead.org>
To: Kees Cook <keescook@...omium.org>
Cc: Linus Torvalds <torvalds@...ux-foundation.org>,
Christian König <christian.koenig@....com>,
Jakob Koschel <jakobkoschel@...il.com>,
alsa-devel@...a-project.org, linux-aspeed@...ts.ozlabs.org,
"Gustavo A. R. Silva" <gustavo@...eddedor.com>,
linux-iio@...r.kernel.org, nouveau@...ts.freedesktop.org,
Rasmus Villemoes <linux@...musvillemoes.dk>,
dri-devel <dri-devel@...ts.freedesktop.org>,
Cristiano Giuffrida <c.giuffrida@...nl>,
amd-gfx list <amd-gfx@...ts.freedesktop.org>,
samba-technical@...ts.samba.org,
linux1394-devel@...ts.sourceforge.net, drbd-dev@...ts.linbit.com,
linux-arch <linux-arch@...r.kernel.org>,
CIFS <linux-cifs@...r.kernel.org>,
KVM list <kvm@...r.kernel.org>,
linux-scsi <linux-scsi@...r.kernel.org>,
linux-rdma <linux-rdma@...r.kernel.org>,
linux-staging@...ts.linux.dev, "Bos, H.J." <h.j.bos@...nl>,
Jason Gunthorpe <jgg@...pe.ca>,
intel-wired-lan@...ts.osuosl.org,
kgdb-bugreport@...ts.sourceforge.net,
bcm-kernel-feedback-list@...adcom.com,
Dan Carpenter <dan.carpenter@...cle.com>,
Linux Media Mailing List <linux-media@...r.kernel.org>,
Arnd Bergman <arnd@...db.de>,
Linux PM <linux-pm@...r.kernel.org>,
intel-gfx <intel-gfx@...ts.freedesktop.org>,
Brian Johannesmeyer <bjohannesmeyer@...il.com>,
Nathan Chancellor <nathan@...nel.org>,
linux-fsdevel <linux-fsdevel@...r.kernel.org>,
Christophe JAILLET <christophe.jaillet@...adoo.fr>,
v9fs-developer@...ts.sourceforge.net,
linux-tegra <linux-tegra@...r.kernel.org>,
Thomas Gleixner <tglx@...utronix.de>,
Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
Linux ARM <linux-arm-kernel@...ts.infradead.org>,
linux-sgx@...r.kernel.org,
linux-block <linux-block@...r.kernel.org>,
Netdev <netdev@...r.kernel.org>, linux-usb@...r.kernel.org,
linux-wireless <linux-wireless@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
Linux F2FS Dev Mailing List
<linux-f2fs-devel@...ts.sourceforge.net>,
tipc-discussion@...ts.sourceforge.net,
Linux Crypto Mailing List <linux-crypto@...r.kernel.org>,
dma <dmaengine@...r.kernel.org>,
linux-mediatek@...ts.infradead.org,
Andrew Morton <akpm@...ux-foundation.org>,
linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>,
Mike Rapoport <rppt@...nel.org>
Subject: Re: [PATCH 2/6] treewide: remove using list iterator after loop body
as a ptr
On Tue, Mar 01, 2022 at 10:14:07AM -0800, Kees Cook wrote:
> On Mon, Feb 28, 2022 at 04:45:11PM -0800, Linus Torvalds wrote:
> > Really. The "-Wshadow doesn't work on the kernel" is not some new
> > issue, because you have to do completely insane things to the source
> > code to enable it.
>
> The first big glitch with -Wshadow was with shadowed global variables.
> GCC 4.8 fixed that, but it still yells about shadowed functions. What
> _almost_ works is -Wshadow=local. At first glace, all the warnings
> look solvable, but then one will eventually discover __wait_event()
> and associated macros that mix when and how deeply it intentionally
> shadows variables. :)
Well, that's just disgusting. Macros fundamentally shouldn't be
referring to things that aren't in their arguments. The first step to
cleaning this up is ...
I'll take a look at the rest of cleaning this up soon.
>From 28ffe35d56223d4242b915832299e5acc926737e Mon Sep 17 00:00:00 2001
From: "Matthew Wilcox (Oracle)" <willy@...radead.org>
Date: Tue, 1 Mar 2022 13:47:07 -0500
Subject: [PATCH] wait: Parameterize the return variable to ___wait_event()
Macros should not refer to variables which aren't in their arguments.
Pass the name from its callers.
Signed-off-by: Matthew Wilcox (Oracle) <willy@...radead.org>
---
include/linux/swait.h | 12 ++++++------
include/linux/wait.h | 32 ++++++++++++++++----------------
include/linux/wait_bit.h | 4 ++--
3 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/include/linux/swait.h b/include/linux/swait.h
index 6a8c22b8c2a5..5e8e9b13be2d 100644
--- a/include/linux/swait.h
+++ b/include/linux/swait.h
@@ -191,14 +191,14 @@ do { \
} while (0)
#define __swait_event_timeout(wq, condition, timeout) \
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_UNINTERRUPTIBLE, timeout, \
__ret = schedule_timeout(__ret))
#define swait_event_timeout_exclusive(wq, condition, timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __swait_event_timeout(wq, condition, timeout); \
__ret; \
})
@@ -216,14 +216,14 @@ do { \
})
#define __swait_event_interruptible_timeout(wq, condition, timeout) \
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_INTERRUPTIBLE, timeout, \
__ret = schedule_timeout(__ret))
#define swait_event_interruptible_timeout_exclusive(wq, condition, timeout)\
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __swait_event_interruptible_timeout(wq, \
condition, timeout); \
__ret; \
@@ -252,7 +252,7 @@ do { \
} while (0)
#define __swait_event_idle_timeout(wq, condition, timeout) \
- ___swait_event(wq, ___wait_cond_timeout(condition), \
+ ___swait_event(wq, ___wait_cond_timeout(condition, __ret), \
TASK_IDLE, timeout, \
__ret = schedule_timeout(__ret))
@@ -278,7 +278,7 @@ do { \
#define swait_event_idle_timeout_exclusive(wq, condition, timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __swait_event_idle_timeout(wq, \
condition, timeout); \
__ret; \
diff --git a/include/linux/wait.h b/include/linux/wait.h
index 851e07da2583..890cce3c0f2e 100644
--- a/include/linux/wait.h
+++ b/include/linux/wait.h
@@ -271,7 +271,7 @@ static inline void wake_up_pollfree(struct wait_queue_head *wq_head)
__wake_up_pollfree(wq_head);
}
-#define ___wait_cond_timeout(condition) \
+#define ___wait_cond_timeout(condition, __ret) \
({ \
bool __cond = (condition); \
if (__cond && !__ret) \
@@ -386,7 +386,7 @@ do { \
})
#define __wait_event_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_UNINTERRUPTIBLE, 0, timeout, \
__ret = schedule_timeout(__ret))
@@ -413,13 +413,13 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_timeout(wq_head, condition, timeout); \
__ret; \
})
#define __wait_event_freezable_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_INTERRUPTIBLE, 0, timeout, \
__ret = freezable_schedule_timeout(__ret))
@@ -431,7 +431,7 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_freezable_timeout(wq_head, condition, timeout); \
__ret; \
})
@@ -503,7 +503,7 @@ do { \
})
#define __wait_event_interruptible_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_INTERRUPTIBLE, 0, timeout, \
__ret = schedule_timeout(__ret))
@@ -531,7 +531,7 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_interruptible_timeout(wq_head, \
condition, timeout); \
__ret; \
@@ -698,7 +698,7 @@ do { \
} while (0)
#define __wait_event_idle_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_IDLE, 0, timeout, \
__ret = schedule_timeout(__ret))
@@ -725,13 +725,13 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_idle_timeout(wq_head, condition, timeout); \
__ret; \
})
#define __wait_event_idle_exclusive_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_IDLE, 1, timeout, \
__ret = schedule_timeout(__ret))
@@ -762,7 +762,7 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_idle_exclusive_timeout(wq_head, condition, timeout);\
__ret; \
})
@@ -932,7 +932,7 @@ extern int do_wait_intr_irq(wait_queue_head_t *, wait_queue_entry_t *);
})
#define __wait_event_killable_timeout(wq_head, condition, timeout) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
TASK_KILLABLE, 0, timeout, \
__ret = schedule_timeout(__ret))
@@ -962,7 +962,7 @@ extern int do_wait_intr_irq(wait_queue_head_t *, wait_queue_entry_t *);
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_killable_timeout(wq_head, \
condition, timeout); \
__ret; \
@@ -1107,7 +1107,7 @@ do { \
})
#define __wait_event_lock_irq_timeout(wq_head, condition, lock, timeout, state) \
- ___wait_event(wq_head, ___wait_cond_timeout(condition), \
+ ___wait_event(wq_head, ___wait_cond_timeout(condition, __ret), \
state, 0, timeout, \
spin_unlock_irq(&lock); \
__ret = schedule_timeout(__ret); \
@@ -1141,7 +1141,7 @@ do { \
timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_lock_irq_timeout( \
wq_head, condition, lock, timeout, \
TASK_INTERRUPTIBLE); \
@@ -1151,7 +1151,7 @@ do { \
#define wait_event_lock_irq_timeout(wq_head, condition, lock, timeout) \
({ \
long __ret = timeout; \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_event_lock_irq_timeout( \
wq_head, condition, lock, timeout, \
TASK_UNINTERRUPTIBLE); \
diff --git a/include/linux/wait_bit.h b/include/linux/wait_bit.h
index 7dec36aecbd9..227e6a20a978 100644
--- a/include/linux/wait_bit.h
+++ b/include/linux/wait_bit.h
@@ -292,7 +292,7 @@ do { \
})
#define __wait_var_event_timeout(var, condition, timeout) \
- ___wait_var_event(var, ___wait_cond_timeout(condition), \
+ ___wait_var_event(var, ___wait_cond_timeout(condition, __ret), \
TASK_UNINTERRUPTIBLE, 0, timeout, \
__ret = schedule_timeout(__ret))
@@ -300,7 +300,7 @@ do { \
({ \
long __ret = timeout; \
might_sleep(); \
- if (!___wait_cond_timeout(condition)) \
+ if (!___wait_cond_timeout(condition, __ret)) \
__ret = __wait_var_event_timeout(var, condition, timeout); \
__ret; \
})
--
2.34.1
Powered by blists - more mailing lists