[RFC PATCH 0/9] fix -Wempty-body build warnings

[RFC PATCH 0/9] fix -Wempty-body build warnings

[Randy Dunlap]

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(-)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[RFC PATCH 0/9] fix -Wempty-body build warnings

[Randy Dunlap]

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(-)

[RFC PATCH 0/9] fix -Wempty-body build warnings

[Randy Dunlap]

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(-)

[RFC PATCH 0/9] fix -Wempty-body build warnings

[Randy Dunlap]

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(-)

[RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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

[RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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

[RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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

[PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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.

[PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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.

[PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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.

[PATCH 3/9] input: fix empty-body warning in synaptics.c

[Randy Dunlap]

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))
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 3/9] input: fix empty-body warning in synaptics.c

[Randy Dunlap]

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))

[PATCH 3/9] input: fix empty-body warning in synaptics.c

[Randy Dunlap]

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))

[PATCH 3/9] input: fix empty-body warning in synaptics.c

[Randy Dunlap]

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))

[PATCH 4/9] sound: fix empty-body warning in vx_core.c

[Randy Dunlap]

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)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 4/9] sound: fix empty-body warning in vx_core.c

[Randy Dunlap]

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)

[PATCH 4/9] sound: fix empty-body warning in vx_core.c

[Randy Dunlap]

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)

[PATCH 4/9] sound: fix empty-body warning in vx_core.c

[Randy Dunlap]

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)

[PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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;
 }
 
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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;
 }
 

[PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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;
 }
 

[PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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;
 }
 

[PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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);

[PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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);

[PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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);

[PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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);

[PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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);

[PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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);

[PATCH 8/9] dax: fix empty-body warnings in bus.c

[Randy Dunlap]

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)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 8/9] dax: fix empty-body warnings in bus.c

[Randy Dunlap]

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)

[PATCH 8/9] dax: fix empty-body warnings in bus.c

[Randy Dunlap]

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)

[PATCH 8/9] dax: fix empty-body warnings in bus.c

[Randy Dunlap]

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)

[PATCH 9/9] target: fix empty-body warning in target_core_pscsi.c

[Randy Dunlap]

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))
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

[PATCH 9/9] target: fix empty-body warning in target_core_pscsi.c

[Randy Dunlap]

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))

[PATCH 9/9] target: fix empty-body warning in target_core_pscsi.c

[Randy Dunlap]

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))

[PATCH 9/9] target: fix empty-body warning in target_core_pscsi.c

[Randy Dunlap]

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))

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Matthew Wilcox]

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. */
> +		do_empty(); /* We don't actually care if the function fails. */
>  	intf->sysfs_files_created = 1;
>  }

Why not just?

+	if (alt->string)
+		device_create_file(&intf->dev, &dev_attr_interface);
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Matthew Wilcox]

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. */
> +		do_empty(); /* We don't actually care if the function fails. */
>  	intf->sysfs_files_created = 1;
>  }

Why not just?

+	if (alt->string)
+		device_create_file(&intf->dev, &dev_attr_interface);

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Matthew Wilcox]

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. */
> +		do_empty(); /* We don't actually care if the function fails. */
>  	intf->sysfs_files_created = 1;
>  }

Why not just?

+	if (alt->string)
+		device_create_file(&intf->dev, &dev_attr_interface);

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Matthew Wilcox]

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. */
> +		do_empty(); /* We don't actually care if the function fails. */
>  	intf->sysfs_files_created = 1;
>  }

Why not just?

+	if (alt->string)
+		device_create_file(&intf->dev, &dev_attr_interface);

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Joe Perches]

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

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Joe Perches]

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

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Joe Perches]

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

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Joe Perches]

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

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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();
> 	new->cl_minorversion = 0;
> 	gen_callback(new, setclid, rqstp);
> 	add_to_unconfirmed(new);

--
Chuck Lever


_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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();
> 	new->cl_minorversion = 0;
> 	gen_callback(new, setclid, rqstp);
> 	add_to_unconfirmed(new);

--
Chuck Lever

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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();
> 	new->cl_minorversion = 0;
> 	gen_callback(new, setclid, rqstp);
> 	add_to_unconfirmed(new);

--
Chuck Lever

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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();
> 	new->cl_minorversion = 0;
> 	gen_callback(new, setclid, rqstp);
> 	add_to_unconfirmed(new);

--
Chuck Lever

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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. */
>> +		do_empty(); /* We don't actually care if the function fails. */
>>  	intf->sysfs_files_created = 1;
>>  }
> 
> Why not just?
> 
> +	if (alt->string)
> +		device_create_file(&intf->dev, &dev_attr_interface);
> 

Yes, looks good. Thanks.

-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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. */
>> +		do_empty(); /* We don't actually care if the function fails. */
>>  	intf->sysfs_files_created = 1;
>>  }
> 
> Why not just?
> 
> +	if (alt->string)
> +		device_create_file(&intf->dev, &dev_attr_interface);
> 

Yes, looks good. Thanks.

-- 
~Randy

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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. */
>> +		do_empty(); /* We don't actually care if the function fails. */
>>  	intf->sysfs_files_created = 1;
>>  }
> 
> Why not just?
> 
> +	if (alt->string)
> +		device_create_file(&intf->dev, &dev_attr_interface);
> 

Yes, looks good. Thanks.

-- 
~Randy

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Randy Dunlap]

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. */
>> +		do_empty(); /* We don't actually care if the function fails. */
>>  	intf->sysfs_files_created = 1;
>>  }
> 
> Why not just?
> 
> +	if (alt->string)
> +		device_create_file(&intf->dev, &dev_attr_interface);
> 

Yes, looks good. Thanks.

-- 
~Randy

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.

-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.

-- 
~Randy

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.

-- 
~Randy

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.

-- 
~Randy

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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");
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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");

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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");

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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");

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Linus Torvalds]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Linus Torvalds]

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

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Linus Torvalds]

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

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Linus Torvalds]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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.
-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Joe Perches]

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();
> > 	new->cl_minorversion = 0;
> > 	gen_callback(new, setclid, rqstp);
> > 	add_to_unconfirmed(new);

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.

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Joe Perches]

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();
> > 	new->cl_minorversion = 0;
> > 	gen_callback(new, setclid, rqstp);
> > 	add_to_unconfirmed(new);

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.

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Joe Perches]

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();
> > 	new->cl_minorversion = 0;
> > 	gen_callback(new, setclid, rqstp);
> > 	add_to_unconfirmed(new);

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.

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Joe Perches]

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();
> > 	new->cl_minorversion = 0;
> > 	gen_callback(new, setclid, rqstp);
> > 	add_to_unconfirmed(new);

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.

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Joe Perches]

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

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Joe Perches]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Joe Perches]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Joe Perches]

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

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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. :(

-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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. :(

-- 
~Randy

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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. :(

-- 
~Randy

Re: [PATCH 2/9] fs: fix empty-body warning in posix_acl.c

[Randy Dunlap]

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. :(

-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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): */
>>> -		;
>>> +		do_empty();
>>> 	new->cl_minorversion = 0;
>>> 	gen_callback(new, setclid, rqstp);
>>> 	add_to_unconfirmed(new);
> 
> 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.
-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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): */
>>> -		;
>>> +		do_empty();
>>> 	new->cl_minorversion = 0;
>>> 	gen_callback(new, setclid, rqstp);
>>> 	add_to_unconfirmed(new);
> 
> 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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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): */
>>> -		;
>>> +		do_empty();
>>> 	new->cl_minorversion = 0;
>>> 	gen_callback(new, setclid, rqstp);
>>> 	add_to_unconfirmed(new);
> 
> 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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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): */
>>> -		;
>>> +		do_empty();
>>> 	new->cl_minorversion = 0;
>>> 	gen_callback(new, setclid, rqstp);
>>> 	add_to_unconfirmed(new);
> 
> 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.
-- 
~Randy

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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.

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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.

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Matthew Wilcox]

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.

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Linus Torvalds]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Linus Torvalds]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Linus Torvalds]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Linus Torvalds]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Johannes Berg]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Johannes Berg]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Johannes Berg]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Johannes Berg]

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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> > +		do_empty(); /* We don't actually care if the function fails. */
> >  	intf->sysfs_files_created = 1;
> >  }
> 
> 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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> > +		do_empty(); /* We don't actually care if the function fails. */
> >  	intf->sysfs_files_created = 1;
> >  }
> 
> 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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> > +		do_empty(); /* We don't actually care if the function fails. */
> >  	intf->sysfs_files_created = 1;
> >  }
> 
> 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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> > +		do_empty(); /* We don't actually care if the function fails. */
> >  	intf->sysfs_files_created = 1;
> >  }
> 
> 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

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Bart Van Assche]

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.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Bart Van Assche]

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.

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Bart Van Assche]

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.

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Bart Van Assche]

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.

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.
-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.
-- 
~Randy

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.
-- 
~Randy

Re: [RFC PATCH 1/9] kernel.h: add do_empty() macro

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Trond Myklebust]

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
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com


_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Trond Myklebust]

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
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Trond Myklebust]

T24gU2F0LCAyMDIwLTA0LTE4IGF0IDE0OjQ1IC0wNDAwLCBDaHVjayBMZXZlciB3cm90ZToNCj4g
PiBPbiBBcHIgMTgsIDIwMjAsIGF0IDI6NDEgUE0sIFJhbmR5IER1bmxhcCA8cmR1bmxhcEBpbmZy
YWRlYWQub3JnPg0KPiA+IHdyb3RlOg0KPiA+IA0KPiA+IEZpeCBnY2MgZW1wdHktYm9keSB3YXJu
aW5nIHdoZW4gLVdleHRyYSBpcyB1c2VkOg0KPiA+IA0KPiA+IC4uL2ZzL25mc2QvbmZzNHN0YXRl
LmM6Mzg5ODozOiB3YXJuaW5nOiBzdWdnZXN0IGJyYWNlcyBhcm91bmQgZW1wdHkNCj4gPiBib2R5
IGluIGFuIOKAmGVsc2XigJkgc3RhdGVtZW50IFstV2VtcHR5LWJvZHldDQo+ID4gDQo+ID4gU2ln
bmVkLW9mZi1ieTogUmFuZHkgRHVubGFwIDxyZHVubGFwQGluZnJhZGVhZC5vcmc+DQo+ID4gQ2M6
IExpbnVzIFRvcnZhbGRzIDx0b3J2YWxkc0BsaW51eC1mb3VuZGF0aW9uLm9yZz4NCj4gPiBDYzog
QW5kcmV3IE1vcnRvbiA8YWtwbUBsaW51eC1mb3VuZGF0aW9uLm9yZz4NCj4gPiBDYzogIkouIEJy
dWNlIEZpZWxkcyIgPGJmaWVsZHNAZmllbGRzZXMub3JnPg0KPiA+IENjOiBDaHVjayBMZXZlciA8
Y2h1Y2subGV2ZXJAb3JhY2xlLmNvbT4NCj4gPiBDYzogbGludXgtbmZzQHZnZXIua2VybmVsLm9y
Zw0KPiANCj4gSSBoYXZlIGEgcGF0Y2ggaW4gbXkgcXVldWUgdGhhdCBhZGRyZXNzZXMgdGhpcyBw
YXJ0aWN1bGFyIHdhcm5pbmcsDQo+IGJ1dCB5b3VyIGNoYW5nZSB3b3JrcyBmb3IgbWUgdG9vLg0K
PiANCj4gQWNrZWQtYnk6IENodWNrIExldmVyIDxjaHVjay5sZXZlckBvcmFjbGUuY29tPg0KPiAN
Cj4gVW5sZXNzIEJydWNlIG9iamVjdHMuDQo+IA0KPiANCj4gPiAtLS0NCj4gPiBmcy9uZnNkL25m
czRzdGF0ZS5jIHwgICAgMyArKy0NCj4gPiAxIGZpbGUgY2hhbmdlZCwgMiBpbnNlcnRpb25zKCsp
LCAxIGRlbGV0aW9uKC0pDQo+ID4gDQo+ID4gLS0tIGxpbnV4LW5leHQtMjAyMDA0MTcub3JpZy9m
cy9uZnNkL25mczRzdGF0ZS5jDQo+ID4gKysrIGxpbnV4LW5leHQtMjAyMDA0MTcvZnMvbmZzZC9u
ZnM0c3RhdGUuYw0KPiA+IEBAIC0zNCw2ICszNCw3IEBADQo+ID4gDQo+ID4gI2luY2x1ZGUgPGxp
bnV4L2ZpbGUuaD4NCj4gPiAjaW5jbHVkZSA8bGludXgvZnMuaD4NCj4gPiArI2luY2x1ZGUgPGxp
bnV4L2tlcm5lbC5oPg0KPiA+ICNpbmNsdWRlIDxsaW51eC9zbGFiLmg+DQo+ID4gI2luY2x1ZGUg
PGxpbnV4L25hbWVpLmg+DQo+ID4gI2luY2x1ZGUgPGxpbnV4L3N3YXAuaD4NCj4gPiBAQCAtMzg5
NSw3ICszODk2LDcgQEAgbmZzZDRfc2V0Y2xpZW50aWQoc3RydWN0IHN2Y19ycXN0ICpycXN0cA0K
PiA+IAkJY29weV9jbGlkKG5ldywgY29uZik7DQo+ID4gCQlnZW5fY29uZmlybShuZXcsIG5uKTsN
Cj4gPiAJfSBlbHNlIC8qIGNhc2UgNCAobmV3IGNsaWVudCkgb3IgY2FzZXMgMiwgMyAoY2xpZW50
IHJlYm9vdCk6ICovDQo+ID4gLQkJOw0KPiA+ICsJCWRvX2VtcHR5KCk7DQoNClVyZ2guLi4gVGhp
cyBpcyBqdXN0IGZvciBkb2N1bWVudGF0aW9uIHB1cnBvc2VzIGFueXdheSwgc28gd2h5IG5vdCBq
dXN0DQp0dXJuIGl0IGFsbCBpbnRvIGEgY29tbWVudCBieSBtb3ZpbmcgdGhlICdlbHNlJyBpbnRv
IHRoZSBjb21tZW50IGZpZWxkPw0KDQppLmUuDQoJfSAvKiBlbHNlIGNhc2UgNCAoLi4uLiAqLw0K
DQoJbmV3LT5jbF9taW5vcnZlcnNpb24gPSAwOw0KPiA+IAlnZW5fY2FsbGJhY2sobmV3LCBzZXRj
bGlkLCBycXN0cCk7DQo+ID4gCWFkZF90b191bmNvbmZpcm1lZChuZXcpOw0KPiANCj4gLS0NCj4g
Q2h1Y2sgTGV2ZXINCj4gDQo+IA0KDQotLSANClRyb25kIE15a2xlYnVzdA0KTGludXggTkZTIGNs
aWVudCBtYWludGFpbmVyLCBIYW1tZXJzcGFjZQ0KdHJvbmQubXlrbGVidXN0QGhhbW1lcnNwYWNl
LmNvbQ0KDQoNCg=

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Trond Myklebust]

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
> 
> 

-- 
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@hammerspace.com

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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.
-- 
~Randy
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Randy Dunlap]

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.
-- 
~Randy

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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


_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Chuck Lever]

> 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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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...
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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...

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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...

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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...

Re: [PATCH 8/9] dax: fix empty-body warnings in bus.c

[Christoph Hellwig]

On Sat, Apr 18, 2020 at 11:41:10AM -0700, Randy Dunlap wrote:
>  				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 */

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.
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 8/9] dax: fix empty-body warnings in bus.c

[Christoph Hellwig]

On Sat, Apr 18, 2020 at 11:41:10AM -0700, Randy Dunlap wrote:
>  				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 */

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.

Re: [PATCH 8/9] dax: fix empty-body warnings in bus.c

[Christoph Hellwig]

On Sat, Apr 18, 2020 at 11:41:10AM -0700, Randy Dunlap wrote:
>  				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 */

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.

Re: [PATCH 8/9] dax: fix empty-body warnings in bus.c

[Christoph Hellwig]

On Sat, Apr 18, 2020 at 11:41:10AM -0700, Randy Dunlap wrote:
>  				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 */

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.

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Sergei Shtylyov]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Sergei Shtylyov]

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

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Sergei Shtylyov]

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

Re: [PATCH 6/9] nfsd: fix empty-body warning in nfs4state.c

[Sergei Shtylyov]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 7/9] drivers/base: fix empty-body warnings in devcoredump.c

[Greg Kroah-Hartman]

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

Re: [PATCH 2/9] fix empty-body warning in posix_acl.c

[Zzy Wysm]

> 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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 2/9] fix empty-body warning in posix_acl.c

[Zzy Wysm]

> 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

Re: [PATCH 2/9] fix empty-body warning in posix_acl.c

[Zzy Wysm]

> 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

Re: [PATCH 2/9] fix empty-body warning in posix_acl.c

[Zzy Wysm]

> 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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[NeilBrown]

[-- Attachment #1.1: Type: text/plain, Size: 1863 bytes --]

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. */
>> > +		do_empty(); /* We don't actually care if the function fails. */
>> >  	intf->sysfs_files_created = 1;
>> >  }
>> 
>> 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

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 167 bytes --]

_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

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. */
>> > +		do_empty(); /* We don't actually care if the function fails. */
>> >  	intf->sysfs_files_created = 1;
>> >  }
>> 
>> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

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. */
>> > +		do_empty(); /* We don't actually care if the function fails. */
>> >  	intf->sysfs_files_created = 1;
>> >  }
>> 
>> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 1863 bytes --]

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. */
>> > +		do_empty(); /* We don't actually care if the function fails. */
>> >  	intf->sysfs_files_created = 1;
>> >  }
>> 
>> 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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> >> > +		do_empty(); /* We don't actually care if the function fails. */
> >> >  	intf->sysfs_files_created = 1;
> >> >  }
> >> 
> >> 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
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> >> > +		do_empty(); /* We don't actually care if the function fails. */
> >> >  	intf->sysfs_files_created = 1;
> >> >  }
> >> 
> >> 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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> >> > +		do_empty(); /* We don't actually care if the function fails. */
> >> >  	intf->sysfs_files_created = 1;
> >> >  }
> >> 
> >> 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

Re: [PATCH 5/9] usb: fix empty-body warning in sysfs.c

[Alan Stern]

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. */
> >> > +		do_empty(); /* We don't actually care if the function fails. */
> >> >  	intf->sysfs_files_created = 1;
> >> >  }
> >> 
> >> 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