* [PATCH] devcoredump: avoid -Wempty-body warnings
@ 2021-03-22 10:19 Arnd Bergmann
2021-03-22 10:29 ` Greg Kroah-Hartman
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-03-22 10:19 UTC (permalink / raw)
To: Johannes Berg, Greg Kroah-Hartman
Cc: Arnd Bergmann, Randy Dunlap, Matthew Wilcox, Rafael J. Wysocki,
Joe Perches, linux-kernel
From: Arnd Bergmann <arnd@arndb.de>
Cleaning out the last -Wempty-body warnings found some interesting
cases with empty macros, along with harmless warnings like this one:
drivers/base/devcoredump.c: In function 'dev_coredumpm':
drivers/base/devcoredump.c:297:56: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
297 | /* nothing - symlink will be missing */;
| ^
drivers/base/devcoredump.c:301:56: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
301 | /* nothing - symlink will be missing */;
| ^
Randy tried addressing this one before, and there were multiple
other ideas in that thread.
Pick up the one that Matthew Wilcox suggested by adding a
WARN_ON_ONCE() and a comment.
Link: https://lore.kernel.org/lkml/20200418184111.13401-8-rdunlap@infradead.org/
Cc: Randy Dunlap <rdunlap@infradead.org>
Cc: Matthew Wilcox <willy@infradead.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/base/devcoredump.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
index 9243468e2c99..88e0d931439e 100644
--- a/drivers/base/devcoredump.c
+++ b/drivers/base/devcoredump.c
@@ -292,13 +292,16 @@ void dev_coredumpm(struct device *dev, struct module *owner,
if (device_add(&devcd->devcd_dev))
goto put_device;
- if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
- "failing_device"))
- /* nothing - symlink will be missing */;
+ /*
+ * These should normally not fail, but there is no problem
+ * continuing without the links, so just warn instead of
+ * failing.
+ */
+ WARN_ON_ONCE(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
+ "failing_device"));
- if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
- "devcoredump"))
- /* nothing - symlink will be missing */;
+ WARN_ON_ONCE(sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
+ "devcoredump"));
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del);
schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
--
2.29.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] devcoredump: avoid -Wempty-body warnings
2021-03-22 10:19 [PATCH] devcoredump: avoid -Wempty-body warnings Arnd Bergmann
@ 2021-03-22 10:29 ` Greg Kroah-Hartman
2021-03-22 11:22 ` Arnd Bergmann
0 siblings, 1 reply; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-22 10:29 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Johannes Berg, Arnd Bergmann, Randy Dunlap, Matthew Wilcox,
Rafael J. Wysocki, Joe Perches, linux-kernel
On Mon, Mar 22, 2021 at 11:19:53AM +0100, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
>
> Cleaning out the last -Wempty-body warnings found some interesting
> cases with empty macros, along with harmless warnings like this one:
>
> drivers/base/devcoredump.c: In function 'dev_coredumpm':
> drivers/base/devcoredump.c:297:56: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> 297 | /* nothing - symlink will be missing */;
> | ^
> drivers/base/devcoredump.c:301:56: error: suggest braces around empty body in an 'if' statement [-Werror=empty-body]
> 301 | /* nothing - symlink will be missing */;
> | ^
>
> Randy tried addressing this one before, and there were multiple
> other ideas in that thread.
>
> Pick up the one that Matthew Wilcox suggested by adding a
> WARN_ON_ONCE() and a comment.
>
> Link: https://lore.kernel.org/lkml/20200418184111.13401-8-rdunlap@infradead.org/
> Cc: Randy Dunlap <rdunlap@infradead.org>
> Cc: Matthew Wilcox <willy@infradead.org>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
> drivers/base/devcoredump.c | 15 +++++++++------
> 1 file changed, 9 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/base/devcoredump.c b/drivers/base/devcoredump.c
> index 9243468e2c99..88e0d931439e 100644
> --- a/drivers/base/devcoredump.c
> +++ b/drivers/base/devcoredump.c
> @@ -292,13 +292,16 @@ void dev_coredumpm(struct device *dev, struct module *owner,
> if (device_add(&devcd->devcd_dev))
> goto put_device;
>
> - if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> - "failing_device"))
> - /* nothing - symlink will be missing */;
> + /*
> + * These should normally not fail, but there is no problem
> + * continuing without the links, so just warn instead of
> + * failing.
> + */
> + WARN_ON_ONCE(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> + "failing_device"));
>
> - if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> - "devcoredump"))
> - /* nothing - symlink will be missing */;
> + WARN_ON_ONCE(sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> + "devcoredump"));
We do not want to reboot machines that have panic-on-warn set, so if
this really needs a trace dump, please do that instead...
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] devcoredump: avoid -Wempty-body warnings
2021-03-22 10:29 ` Greg Kroah-Hartman
@ 2021-03-22 11:22 ` Arnd Bergmann
2021-03-22 11:35 ` Greg Kroah-Hartman
0 siblings, 1 reply; 4+ messages in thread
From: Arnd Bergmann @ 2021-03-22 11:22 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Johannes Berg, Randy Dunlap, Matthew Wilcox, Rafael J. Wysocki,
Joe Perches, Linux Kernel Mailing List
On Mon, Mar 22, 2021 at 11:29 AM Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Mon, Mar 22, 2021 at 11:19:53AM +0100, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > + * These should normally not fail, but there is no problem
> > + * continuing without the links, so just warn instead of
> > + * failing.
> > + */
> > + WARN_ON_ONCE(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> > + "failing_device"));
> >
> > - if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> > - "devcoredump"))
> > - /* nothing - symlink will be missing */;
> > + WARN_ON_ONCE(sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> > + "devcoredump"));
>
> We do not want to reboot machines that have panic-on-warn set,
Fair enough.
> so if this really needs a trace dump, please do that instead...
I don't think the backtrace is needed here, if it ever happens it's either
going to be -ENOMEM or completely reproducible. I can instead
do the cast to (void) that Linus suggested, or a simple if(...) dev_warn()
or dev_warn_once() to have some indication of the failure.
Arnd
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] devcoredump: avoid -Wempty-body warnings
2021-03-22 11:22 ` Arnd Bergmann
@ 2021-03-22 11:35 ` Greg Kroah-Hartman
0 siblings, 0 replies; 4+ messages in thread
From: Greg Kroah-Hartman @ 2021-03-22 11:35 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Johannes Berg, Randy Dunlap, Matthew Wilcox, Rafael J. Wysocki,
Joe Perches, Linux Kernel Mailing List
On Mon, Mar 22, 2021 at 12:22:11PM +0100, Arnd Bergmann wrote:
> On Mon, Mar 22, 2021 at 11:29 AM Greg Kroah-Hartman
> <gregkh@linuxfoundation.org> wrote:
> > On Mon, Mar 22, 2021 at 11:19:53AM +0100, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > > + * These should normally not fail, but there is no problem
> > > + * continuing without the links, so just warn instead of
> > > + * failing.
> > > + */
> > > + WARN_ON_ONCE(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
> > > + "failing_device"));
> > >
> > > - if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> > > - "devcoredump"))
> > > - /* nothing - symlink will be missing */;
> > > + WARN_ON_ONCE(sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj,
> > > + "devcoredump"));
> >
> > We do not want to reboot machines that have panic-on-warn set,
>
> Fair enough.
>
> > so if this really needs a trace dump, please do that instead...
>
> I don't think the backtrace is needed here, if it ever happens it's either
> going to be -ENOMEM or completely reproducible. I can instead
> do the cast to (void) that Linus suggested, or a simple if(...) dev_warn()
> or dev_warn_once() to have some indication of the failure.
dev_warn() should be fine I think.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-22 11:36 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-22 10:19 [PATCH] devcoredump: avoid -Wempty-body warnings Arnd Bergmann
2021-03-22 10:29 ` Greg Kroah-Hartman
2021-03-22 11:22 ` Arnd Bergmann
2021-03-22 11:35 ` Greg Kroah-Hartman
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.