[RFC PATCH 0/9] fix -Wempty-body build warnings
Hi,
When -Wextra is used, gcc emits many warnings about an empty 'if' or 'else' body, like this:
../fs/posix_acl.c: In function ‘get_acl’: ../fs/posix_acl.c:127:22: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] /* fall through */ ; ^
To quieten these warnings, add a new macro "do_empty()". I originally wanted to use do_nothing(), but that's already in use.
It would sorta be nice if "fallthrough" could be coerced for this instead of using something like do_empty().
Or should we just use "{}" in place of ";"? This causes some odd coding style issue IMO. E.g., see this change:
original: if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) /* fall through */ ;
with new macro: if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) do_empty(); /* fall through */
using {}: if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) {} /* fall through */ or { /* fall through */ } or even if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) { /* fall through */ } or if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) { } /* fall through */
drivers/base/devcoredump.c | 5 +++-- drivers/dax/bus.c | 5 +++-- drivers/input/mouse/synaptics.c | 3 ++- drivers/target/target_core_pscsi.c | 3 ++- drivers/usb/core/sysfs.c | 2 +- fs/nfsd/nfs4state.c | 3 ++- fs/posix_acl.c | 2 +- include/linux/kernel.h | 8 ++++++++ sound/drivers/vx/vx_core.c | 3 ++- 9 files changed, 24 insertions(+), 10 deletions(-)
Add the do_empty() macro to silence gcc warnings about an empty body following an "if" statement when -Wextra is used.
However, for debug printk calls that are being disabled, use either no_printk() or pr_debug() [and optionally dynamic printk debugging] instead.
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: linux-fsdevel@vger.kernel.org Cc: Dmitry Torokhov dmitry.torokhov@gmail.com Cc: linux-input@vger.kernel.org Cc: Jaroslav Kysela perex@perex.cz Cc: Takashi Iwai tiwai@suse.com Cc: alsa-devel@alsa-project.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org Cc: Johannes Berg johannes@sipsolutions.net Cc: Dan Williams dan.j.williams@intel.com Cc: Vishal Verma vishal.l.verma@intel.com Cc: Dave Jiang dave.jiang@intel.com Cc: linux-nvdimm@lists.01.org Cc: "Martin K. Petersen" martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org Cc: target-devel@vger.kernel.org Cc: Zzy Wysm zzy@zzywysm.com --- include/linux/kernel.h | 8 ++++++++ 1 file changed, 8 insertions(+)
--- linux-next-20200327.orig/include/linux/kernel.h +++ linux-next-20200327/include/linux/kernel.h @@ -40,6 +40,14 @@ #define READ 0 #define WRITE 1
+/* + * When using -Wextra, an "if" statement followed by an empty block + * (containing only a ';'), produces a warning from gcc: + * warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] + * Replace the empty body with do_empty() to silence this warning. + */ +#define do_empty() do { } while (0) + /** * ARRAY_SIZE - get the number of elements in array @arr * @arr: array to be sized
On Sat, 2020-04-18 at 11:41 -0700, Randy Dunlap wrote:
Add the do_empty() macro to silence gcc warnings about an empty body following an "if" statement when -Wextra is used.
However, for debug printk calls that are being disabled, use either no_printk() or pr_debug() [and optionally dynamic printk debugging] instead.
[]
+#define do_empty() do { } while (0)
If this is really useful (I think the warning is somewhat silly)
bikeshed:
I think do_nothing() is more descriptive
On 4/18/20 11:44 AM, Joe Perches wrote:
On Sat, 2020-04-18 at 11:41 -0700, Randy Dunlap wrote:
Add the do_empty() macro to silence gcc warnings about an empty body following an "if" statement when -Wextra is used.
However, for debug printk calls that are being disabled, use either no_printk() or pr_debug() [and optionally dynamic printk debugging] instead.
[]
+#define do_empty() do { } while (0)
If this is really useful (I think the warning is somewhat silly)
bikeshed:
I think do_nothing() is more descriptive
Yes, I do too, as I more or less mentioned in the cover letter.
On 4/18/20 11:41 AM, Randy Dunlap wrote:
--- linux-next-20200327.orig/include/linux/kernel.h +++ linux-next-20200327/include/linux/kernel.h @@ -40,6 +40,14 @@ #define READ 0 #define WRITE 1
+/*
- When using -Wextra, an "if" statement followed by an empty block
- (containing only a ';'), produces a warning from gcc:
- warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
- Replace the empty body with do_empty() to silence this warning.
- */
+#define do_empty() do { } while (0)
- /**
- ARRAY_SIZE - get the number of elements in array @arr
- @arr: array to be sized
I'm less than enthusiast about introducing a new macro to suppress "empty body" warnings. Anyone who encounters code in which this macro is used will have to look up the definition of this macro to learn what it does. Has it been considered to suppress empty body warnings by changing the empty bodies from ";" into "{}"?
Thanks,
Bart.
On 4/18/20 3:20 PM, Bart Van Assche wrote:
On 4/18/20 11:41 AM, Randy Dunlap wrote:
--- linux-next-20200327.orig/include/linux/kernel.h +++ linux-next-20200327/include/linux/kernel.h @@ -40,6 +40,14 @@ #define READ 0 #define WRITE 1 +/*
- When using -Wextra, an "if" statement followed by an empty block
- (containing only a ';'), produces a warning from gcc:
- warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
- Replace the empty body with do_empty() to silence this warning.
- */
+#define do_empty() do { } while (0)
/** * ARRAY_SIZE - get the number of elements in array @arr * @arr: array to be sized
I'm less than enthusiast about introducing a new macro to suppress "empty body" warnings. Anyone who encounters code in which this macro is used will have to look up the definition of this macro to learn what it does. Has it been considered to suppress empty body warnings by changing the empty bodies from ";" into "{}"?
I mentioned that possibility in PATCH 0/9 (cover letter)... which should have been RFC PATCH 0/9. So yes, it is possible.
You are the only other person who has mentioned it.
thanks.
Fix gcc empty-body warning when -Wextra is used:
../fs/posix_acl.c: In function ‘get_acl’: ../fs/posix_acl.c:127:22: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] /* fall through */ ; ^
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Alexander Viro viro@zeniv.linux.org.uk Cc: linux-fsdevel@vger.kernel.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- fs/posix_acl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200327.orig/fs/posix_acl.c +++ linux-next-20200327/fs/posix_acl.c @@ -124,7 +124,7 @@ struct posix_acl *get_acl(struct inode * * be an unlikely race.) */ if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED) - /* fall through */ ; + do_empty(); /* fall through */
/* * Normally, the ACL returned by ->get_acl will be cached.
On Sat, Apr 18, 2020 at 11:41 AM Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
Please don't do this.
First off, "do_empty()" adds nothing but confusion. Now it syntactically looks like it does something, and it's a new pattern to everybody. I've never seen it before.
Secondly, even if we were to do this, then the patch would be wrong:
if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED)
/* fall through */ ;
do_empty(); /* fall through */
That comment made little sense before, but it makes _no_ sense now.
What fall-through? I'm guessing it meant to say "nothing", and somebody was confused. With "do_empty()", it's even more confusing.
Thirdly, there's a *reason* why "-Wextra" isn't used.
The warnings enabled by -Wextra are usually complete garbage, and trying to fix them often makes the code worse. Exactly like here.
Linus
On 4/18/20 11:53 AM, Linus Torvalds wrote:
On Sat, Apr 18, 2020 at 11:41 AM Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
Please don't do this.
First off, "do_empty()" adds nothing but confusion. Now it syntactically looks like it does something, and it's a new pattern to everybody. I've never seen it before.
Secondly, even if we were to do this, then the patch would be wrong:
if (cmpxchg(p, ACL_NOT_CACHED, sentinel) != ACL_NOT_CACHED)
/* fall through */ ;
do_empty(); /* fall through */
That comment made little sense before, but it makes _no_ sense now.
What fall-through? I'm guessing it meant to say "nothing", and somebody was confused. With "do_empty()", it's even more confusing.
Thirdly, there's a *reason* why "-Wextra" isn't used.
The warnings enabled by -Wextra are usually complete garbage, and trying to fix them often makes the code worse. Exactly like here.
OK, no problem. That's why PATCH 0/9 says RFC.
Oops. Crap. It was *supposed* to say RFC. :(
On Apr 18, 2020, at 1:53 PM, Linus Torvalds torvalds@linux-foundation.org wrote:
Thirdly, there's a *reason* why "-Wextra" isn't used.
The warnings enabled by -Wextra are usually complete garbage, and trying to fix them often makes the code worse. Exactly like here.
As the instigator of this warning cleanup activity, even _I_ don’t recommend building with all of -Wextra. Doing so on an allmodconfig build generates 500 megabytes of warning text (not exaggerating), primarily due to -Wunused-parameter and Wmissing-field-initializers.
I strongly recommend disabling them with -Wno-unused-parameter -Wno-missing-field-initializers since the spew is completely unactionable.
On the other hand, -Woverride-init found a legit bug that was breaking DVD drives, fixed in commit 03264ddde2453f6877a7d637d84068079632a3c5.
I believe finding a good set of compiler warning settings can lead to lots of good bugs being spotted and (hopefully) fixed. Why let syzbot do all the work?
zzy
Fix gcc empty-body warning when -Wextra is used:
../drivers/input/mouse/synaptics.c: In function ‘synaptics_process_packet’: ../drivers/input/mouse/synaptics.c:1106:6: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] ; /* Nothing, treat a pen as a single finger */ ^
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Dmitry Torokhov dmitry.torokhov@gmail.com Cc: linux-input@vger.kernel.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- drivers/input/mouse/synaptics.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200327.orig/drivers/input/mouse/synaptics.c +++ linux-next-20200327/drivers/input/mouse/synaptics.c @@ -20,6 +20,7 @@ * Trademarks are the property of their respective owners. */
+#include <linux/kernel.h> #include <linux/module.h> #include <linux/delay.h> #include <linux/dmi.h> @@ -1103,7 +1104,7 @@ static void synaptics_process_packet(str break; case 2: if (SYN_MODEL_PEN(info->model_id)) - ; /* Nothing, treat a pen as a single finger */ + do_empty(); /* Nothing, treat a pen as a single finger */ break; case 4 ... 15: if (SYN_CAP_PALMDETECT(info->capabilities))
Fix gcc empty-body warning when -Wextra is used:
../sound/drivers/vx/vx_core.c:515:3: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Jaroslav Kysela perex@perex.cz Cc: Takashi Iwai tiwai@suse.com Cc: alsa-devel@alsa-project.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- sound/drivers/vx/vx_core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200327.orig/sound/drivers/vx/vx_core.c +++ linux-next-20200327/sound/drivers/vx/vx_core.c @@ -13,6 +13,7 @@ #include <linux/init.h> #include <linux/device.h> #include <linux/firmware.h> +#include <linux/kernel.h> #include <linux/module.h> #include <linux/io.h> #include <sound/core.h> @@ -512,7 +513,7 @@ irqreturn_t snd_vx_threaded_irq_handler( * received by the board is equal to one of those given to it). */ if (events & TIME_CODE_EVENT_PENDING) - ; /* so far, nothing to do yet */ + do_empty(); /* so far, nothing to do yet */
/* The frequency has changed on the board (UER mode). */ if (events & FREQUENCY_CHANGE_EVENT_PENDING)
Fix gcc empty-body warning when -Wextra is used:
../drivers/usb/core/sysfs.c: In function ‘usb_create_sysfs_intf_files’: ../drivers/usb/core/sysfs.c:1266:3: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] ; /* We don't actually care if the function fails. */ ^
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Greg Kroah-Hartman gregkh@linuxfoundation.org Cc: linux-usb@vger.kernel.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- drivers/usb/core/sysfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
--- linux-next-20200327.orig/drivers/usb/core/sysfs.c +++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface)) - ; /* We don't actually care if the function fails. */ + do_empty(); /* We don't actually care if the function fails. */ intf->sysfs_files_created = 1; }
On Sat, Apr 18, 2020 at 11:41:07AM -0700, Randy Dunlap wrote:
+++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface))
; /* We don't actually care if the function fails. */
intf->sysfs_files_created = 1;do_empty(); /* We don't actually care if the function fails. */
}
Why not just?
+ if (alt->string) + device_create_file(&intf->dev, &dev_attr_interface);
On 4/18/20 11:44 AM, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:07AM -0700, Randy Dunlap wrote:
+++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface))
; /* We don't actually care if the function fails. */
intf->sysfs_files_created = 1;do_empty(); /* We don't actually care if the function fails. */
}
Why not just?
- if (alt->string)
device_create_file(&intf->dev, &dev_attr_interface);
Yes, looks good. Thanks.
On Sat, 18 Apr 2020, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:07AM -0700, Randy Dunlap wrote:
+++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface))
; /* We don't actually care if the function fails. */
intf->sysfs_files_created = 1;do_empty(); /* We don't actually care if the function fails. */
}
Why not just?
- if (alt->string)
device_create_file(&intf->dev, &dev_attr_interface);
This is another __must_check function call.
The reason we don't care if the call fails is because the file being created holds the USB interface string descriptor, something which is purely informational and hardly ever gets set (and no doubt gets used even less often).
Is this another situation where the comment should be expanded and the code modified to include a useless test and cast-to-void?
Or should device_create_file() not be __must_check after all?
Greg?
Alan Stern
On Sat, Apr 18 2020, Alan Stern wrote:
On Sat, 18 Apr 2020, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:07AM -0700, Randy Dunlap wrote:
+++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface))
; /* We don't actually care if the function fails. */
intf->sysfs_files_created = 1;do_empty(); /* We don't actually care if the function fails. */
}
Why not just?
- if (alt->string)
device_create_file(&intf->dev, &dev_attr_interface);
This is another __must_check function call.
The reason we don't care if the call fails is because the file being created holds the USB interface string descriptor, something which is purely informational and hardly ever gets set (and no doubt gets used even less often).
Is this another situation where the comment should be expanded and the code modified to include a useless test and cast-to-void?
Or should device_create_file() not be __must_check after all?
One approach to dealing with __must_check function that you don't want to check is to cause failure to call pr_debug("usb: interface descriptor file not created"); or similar. It silences the compiler, serves as documentation, and creates a message that is almost certainly never seen.
This is what I did in drivers/md/md.c...
if (mddev->kobj.sd && sysfs_create_group(&mddev->kobj, &md_bitmap_group)) pr_debug("pointless warning\n");
(I give better warnings elsewhere - I must have run out of patience by this point).
NeilBrown
On Tue, 21 Apr 2020, NeilBrown wrote:
On Sat, Apr 18 2020, Alan Stern wrote:
On Sat, 18 Apr 2020, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:07AM -0700, Randy Dunlap wrote:
+++ linux-next-20200327/drivers/usb/core/sysfs.c @@ -1263,7 +1263,7 @@ void usb_create_sysfs_intf_files(struct if (!alt->string && !(udev->quirks & USB_QUIRK_CONFIG_INTF_STRINGS)) alt->string = usb_cache_string(udev, alt->desc.iInterface); if (alt->string && device_create_file(&intf->dev, &dev_attr_interface))
; /* We don't actually care if the function fails. */
intf->sysfs_files_created = 1;do_empty(); /* We don't actually care if the function fails. */
}
Why not just?
- if (alt->string)
device_create_file(&intf->dev, &dev_attr_interface);
This is another __must_check function call.
The reason we don't care if the call fails is because the file being created holds the USB interface string descriptor, something which is purely informational and hardly ever gets set (and no doubt gets used even less often).
Is this another situation where the comment should be expanded and the code modified to include a useless test and cast-to-void?
Or should device_create_file() not be __must_check after all?
One approach to dealing with __must_check function that you don't want to check is to cause failure to call pr_debug("usb: interface descriptor file not created"); or similar. It silences the compiler, serves as documentation, and creates a message that is almost certainly never seen.
This is what I did in drivers/md/md.c...
if (mddev->kobj.sd && sysfs_create_group(&mddev->kobj, &md_bitmap_group)) pr_debug("pointless warning\n");
(I give better warnings elsewhere - I must have run out of patience by this point).
That's a decent idea. I'll do something along those lines.
Alan Stern
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org --- fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */ - ; + do_empty(); new->cl_minorversion = 0; gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
new->cl_minorversion = 0; gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);do_empty();
-- Chuck Lever
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote:
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
new->cl_minorversion = 0; gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);do_empty();
This empty else seems silly and could likely be better handled by a comment above the first if, something like:
/* for now only handle case 1: probable callback update */ if (conf && same_verf(&conf->cl_verifier, &clverifier)) { copy_clid(new, conf); gen_confirm(new, nn); }
with no else use.
On 4/18/20 11:53 AM, Joe Perches wrote:
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote:
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
new->cl_minorversion = 0; gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);do_empty();
This empty else seems silly and could likely be better handled by a comment above the first if, something like:
/* for now only handle case 1: probable callback update */ if (conf && same_verf(&conf->cl_verifier, &clverifier)) { copy_clid(new, conf); gen_confirm(new, nn); }
with no else use.
I'll just let Chuck handle it with his current patch, whatever it is.
thanks.
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote:
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
do_empty();
Urgh... This is just for documentation purposes anyway, so why not just turn it all into a comment by moving the 'else' into the comment field?
i.e. } /* else case 4 (.... */
new->cl_minorversion = 0;
gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);
-- Chuck Lever
On 4/18/20 3:28 PM, Trond Myklebust wrote:
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote:
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
do_empty();
Urgh... This is just for documentation purposes anyway, so why not just turn it all into a comment by moving the 'else' into the comment field?
i.e. } /* else case 4 (.... */
new->cl_minorversion = 0;
gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);
Like I said earlier, since Chuck has a patch that addresses this, let's just go with that.
thanks.
On Apr 18, 2020, at 6:32 PM, Randy Dunlap rdunlap@infradead.org wrote:
On 4/18/20 3:28 PM, Trond Myklebust wrote:
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote:
On Apr 18, 2020, at 2:41 PM, Randy Dunlap rdunlap@infradead.org wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
I have a patch in my queue that addresses this particular warning, but your change works for me too.
Acked-by: Chuck Lever chuck.lever@oracle.com
Unless Bruce objects.
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c @@ -34,6 +34,7 @@
#include <linux/file.h> #include <linux/fs.h> +#include <linux/kernel.h> #include <linux/slab.h> #include <linux/namei.h> #include <linux/swap.h> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
do_empty();
Urgh... This is just for documentation purposes anyway, so why not just turn it all into a comment by moving the 'else' into the comment field?
i.e. } /* else case 4 (.... */
new->cl_minorversion = 0;
gen_callback(new, setclid, rqstp); add_to_unconfirmed(new);
Like I said earlier, since Chuck has a patch that addresses this, let's just go with that.
I'll post that patch for review as part of my NFSD for-5.8 patches.
-- Chuck Lever
Hello!
On 18.04.2020 21:41, Randy Dunlap wrote:
Fix gcc empty-body warning when -Wextra is used:
../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "J. Bruce Fields" bfields@fieldses.org Cc: Chuck Lever chuck.lever@oracle.com Cc: linux-nfs@vger.kernel.org
fs/nfsd/nfs4state.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/fs/nfsd/nfs4state.c +++ linux-next-20200417/fs/nfsd/nfs4state.c
[...]
@@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp copy_clid(new, conf); gen_confirm(new, nn); } else /* case 4 (new client) or cases 2, 3 (client reboot): */
;
do_empty();
In this case explicit {} would probably have been better, as described in Documentation/process/coding-style.rst, clause (3).
MBR, Sergei
Fix gcc empty-body warning when -Wextra is used:
../drivers/base/devcoredump.c:297:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] ../drivers/base/devcoredump.c:301:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Johannes Berg johannes@sipsolutions.net Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- drivers/base/devcoredump.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
--- linux-next-20200417.orig/drivers/base/devcoredump.c +++ linux-next-20200417/drivers/base/devcoredump.c @@ -9,6 +9,7 @@ * * Author: Johannes Berg johannes@sipsolutions.net */ +#include <linux/kernel.h> #include <linux/module.h> #include <linux/device.h> #include <linux/devcoredump.h> @@ -294,11 +295,11 @@ void dev_coredumpm(struct device *dev, s
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device")) - /* nothing - symlink will be missing */; + do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump")) - /* nothing - symlink will be missing */; + do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
@@ -294,11 +295,11 @@ void dev_coredumpm(struct device *dev, s
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
Could just remove the 'if's?
+ sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, + "failing_device");
On 4/18/20 11:50 AM, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
@@ -294,11 +295,11 @@ void dev_coredumpm(struct device *dev, s
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
Could just remove the 'if's?
- sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
"failing_device");
OK.
thanks.
On Sat, 2020-04-18 at 11:53 -0700, Randy Dunlap wrote:
On 4/18/20 11:50 AM, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
@@ -294,11 +295,11 @@ void dev_coredumpm(struct device *dev, s
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
Could just remove the 'if's?
- sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
"failing_device");
OK.
sysfs_create_link is __must_check
On Sat, Apr 18, 2020 at 11:55:05AM -0700, Joe Perches wrote:
On Sat, 2020-04-18 at 11:53 -0700, Randy Dunlap wrote:
On 4/18/20 11:50 AM, Matthew Wilcox wrote:
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
@@ -294,11 +295,11 @@ void dev_coredumpm(struct device *dev, s
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
Could just remove the 'if's?
- sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
"failing_device");
OK.
sysfs_create_link is __must_check
Oh, I missed the declaration -- I just saw the definition. This is a situation where __must_check hurts us and it should be removed.
Or this code is wrong and it should be
WARN(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device");
like drivers/pci/controller/vmd.c and drivers/i2c/i2c-mux.c
Either way, the do_empty() construct feels like the wrong way of covering up the warning.
On Sat, 2020-04-18 at 12:13 -0700, Matthew Wilcox wrote:
if (sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
if (sysfs_create_link(&dev->kobj, &devcd->devcd_dev.kobj, "devcoredump"))
/* nothing - symlink will be missing */;
do_empty(); /* nothing - symlink will be missing */
INIT_DELAYED_WORK(&devcd->del_wk, devcd_del); schedule_delayed_work(&devcd->del_wk, DEVCD_TIMEOUT);
Could just remove the 'if's?
- sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj,
"failing_device");
OK.
sysfs_create_link is __must_check
Oh, I missed the declaration -- I just saw the definition. This is a situation where __must_check hurts us and it should be removed.
Or this code is wrong and it should be
WARN(sysfs_create_link(&devcd->devcd_dev.kobj, &dev->kobj, "failing_device");
Perhaps it should be. I didn't think it really mattered _that_ much if the symlink suddenly went missing, but OTOH I don't even know how it could possibly fail.
johannes
On Sat, Apr 18, 2020 at 11:57 AM Joe Perches joe@perches.com wrote:
sysfs_create_link is __must_check
The way to handle __must_check if you really really don't want to test and have good reasons is
(a) add a big comment about why this case ostensibly doesn't need the check
(b) cast a test of it to '(void)' or something (I guess we could add a helper for this). So something like
/* We will always clean up, we don't care whether this fails or succeeds */ (void)!!sysfs_create_link(...)
There are other alternatives (like using WARN_ON_ONCE() instead, for example). So it depends on the code. Which is why that comment is important to show why the code chose that option.
However, I wonder if in this case we should just remove the __must_check. Greg? It goes back a long long time.
Particularly for the "nowarn" version of that function. I'm not seeing why you'd have to care, particularly if you don't even care about the link already existing..
Linus
On Sat, Apr 18, 2020 at 12:15:57PM -0700, Linus Torvalds wrote:
On Sat, Apr 18, 2020 at 11:57 AM Joe Perches joe@perches.com wrote:
sysfs_create_link is __must_check
The way to handle __must_check if you really really don't want to test and have good reasons is
(a) add a big comment about why this case ostensibly doesn't need the check
(b) cast a test of it to '(void)' or something (I guess we could add a helper for this). So something like
/* We will always clean up, we don't care whether this fails
or succeeds */ (void)!!sysfs_create_link(...)
There are other alternatives (like using WARN_ON_ONCE() instead, for example). So it depends on the code. Which is why that comment is important to show why the code chose that option.
However, I wonder if in this case we should just remove the __must_check. Greg? It goes back a long long time.
Yeah, maybe it is time to remove it, the gyrations people go through to remove the warning when they "know" they are doing it right feels pretty bad compared to forcing people to check things for "normal" calls to the function.
thanks,
greg k-h
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
Fix gcc empty-body warning when -Wextra is used:
../drivers/base/devcoredump.c:297:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] ../drivers/base/devcoredump.c:301:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Johannes Berg johannes@sipsolutions.net Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org
drivers/base/devcoredump.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
--- linux-next-20200417.orig/drivers/base/devcoredump.c +++ linux-next-20200417/drivers/base/devcoredump.c @@ -9,6 +9,7 @@
- Author: Johannes Berg johannes@sipsolutions.net
*/ +#include <linux/kernel.h>
Why the need for this .h file being added for reformatting the code?
thanks,
greg k-h
On Sun, Apr 19, 2020 at 08:02:47AM +0200, Greg Kroah-Hartman wrote:
On Sat, Apr 18, 2020 at 11:41:09AM -0700, Randy Dunlap wrote:
Fix gcc empty-body warning when -Wextra is used:
../drivers/base/devcoredump.c:297:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body] ../drivers/base/devcoredump.c:301:42: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Johannes Berg johannes@sipsolutions.net Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org
drivers/base/devcoredump.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
--- linux-next-20200417.orig/drivers/base/devcoredump.c +++ linux-next-20200417/drivers/base/devcoredump.c @@ -9,6 +9,7 @@
- Author: Johannes Berg johannes@sipsolutions.net
*/ +#include <linux/kernel.h>
Why the need for this .h file being added for reformatting the code?
Ah, the function you add, nevermind, need more coffee...
Fix gcc empty-body warning when -Wextra is used:
../drivers/dax/bus.c:93:27: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body] ../drivers/dax/bus.c:98:29: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Dan Williams dan.j.williams@intel.com Cc: Vishal Verma vishal.l.verma@intel.com Cc: Dave Jiang dave.jiang@intel.com Cc: linux-nvdimm@lists.01.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org --- drivers/dax/bus.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)
--- linux-next-20200417.orig/drivers/dax/bus.c +++ linux-next-20200417/drivers/dax/bus.c @@ -2,6 +2,7 @@ /* Copyright(c) 2017-2018 Intel Corporation. All rights reserved. */ #include <linux/memremap.h> #include <linux/device.h> +#include <linux/kernel.h> #include <linux/mutex.h> #include <linux/list.h> #include <linux/slab.h> @@ -90,12 +91,12 @@ static ssize_t do_id_store(struct device } else rc = -ENOMEM; } else - /* nothing to remove */; + do_empty(); /* nothing to remove */ } else if (action == ID_REMOVE) { list_del(&dax_id->list); kfree(dax_id); } else - /* dax_id already added */; + do_empty(); /* dax_id already added */ mutex_unlock(&dax_bus_lock);
if (rc < 0)
On Sat, Apr 18, 2020 at 11:41:10AM -0700, Randy Dunlap wrote:
rc = -ENOMEM; } else
/* nothing to remove */;
} else if (action == ID_REMOVE) { list_del(&dax_id->list); kfree(dax_id); } elsedo_empty(); /* nothing to remove */
/* dax_id already added */;
do_empty(); /* dax_id already added */
This is just nasty. Please just always turn this bogus warning off as the existing code is a perfectly readable idiom while the new code is just nasty crap for no good reason at all.
Fix gcc empty-body warning when -Wextra is used:
../drivers/target/target_core_pscsi.c:624:5: warning: suggest braces around empty body in an ‘if’ statement [-Wempty-body]
Signed-off-by: Randy Dunlap rdunlap@infradead.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Andrew Morton akpm@linux-foundation.org Cc: "Martin K. Petersen" martin.petersen@oracle.com Cc: linux-scsi@vger.kernel.org Cc: target-devel@vger.kernel.org --- drivers/target/target_core_pscsi.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
--- linux-next-20200417.orig/drivers/target/target_core_pscsi.c +++ linux-next-20200417/drivers/target/target_core_pscsi.c @@ -18,6 +18,7 @@ #include <linux/slab.h> #include <linux/spinlock.h> #include <linux/genhd.h> +#include <linux/kernel.h> #include <linux/cdrom.h> #include <linux/ratelimit.h> #include <linux/module.h> @@ -621,7 +622,7 @@ static void pscsi_complete_cmd(struct se
buf = transport_kmap_data_sg(cmd); if (!buf) - ; /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */ + do_empty(); /* XXX: TCM_LOGICAL_UNIT_COMMUNICATION_FAILURE */
if (cdb[0] == MODE_SENSE_10) { if (!(buf[3] & 0x80))
participants (14)
-
Alan Stern
-
Bart Van Assche
-
Christoph Hellwig
-
Chuck Lever
-
Greg Kroah-Hartman
-
Joe Perches
-
Johannes Berg
-
Linus Torvalds
-
Matthew Wilcox
-
NeilBrown
-
Randy Dunlap
-
Sergei Shtylyov
-
Trond Myklebust
-
Zzy Wysm