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

Message ID 20200418184111.13401-7-rdunlap@infradead.org
State New
Headers show
Series
  • fix -Wempty-body build warnings
Related show

Commit Message

Randy Dunlap April 18, 2020, 6:41 p.m. UTC
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(-)

Comments

Chuck Lever April 18, 2020, 6:45 p.m. UTC | #1
> 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
Joe Perches April 18, 2020, 6:53 p.m. UTC | #2
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.
Randy Dunlap April 18, 2020, 6:57 p.m. UTC | #3
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.
Trond Myklebust April 18, 2020, 10:28 p.m. UTC | #4
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
> 
>
Randy Dunlap April 18, 2020, 10:32 p.m. UTC | #5
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.
Chuck Lever April 18, 2020, 10:33 p.m. UTC | #6
> 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
Sergei Shtylyov April 19, 2020, 9:32 a.m. UTC | #7
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

Patch
diff mbox series

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