| Message ID | 20200418184111.13401-7-rdunlap@infradead.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | fix -Wempty-body build warnings | expand |
> 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
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.
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.
On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote: > > On Apr 18, 2020, at 2:41 PM, Randy Dunlap <rdunlap@infradead.org> > > wrote: > > > > Fix gcc empty-body warning when -Wextra is used: > > > > ../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty > > body in an ‘else’ statement [-Wempty-body] > > > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > > Cc: Linus Torvalds <torvalds@linux-foundation.org> > > Cc: Andrew Morton <akpm@linux-foundation.org> > > Cc: "J. Bruce Fields" <bfields@fieldses.org> > > Cc: Chuck Lever <chuck.lever@oracle.com> > > Cc: linux-nfs@vger.kernel.org > > I have a patch in my queue that addresses this particular warning, > but your change works for me too. > > Acked-by: Chuck Lever <chuck.lever@oracle.com> > > Unless Bruce objects. > > > > --- > > fs/nfsd/nfs4state.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > --- linux-next-20200417.orig/fs/nfsd/nfs4state.c > > +++ linux-next-20200417/fs/nfsd/nfs4state.c > > @@ -34,6 +34,7 @@ > > > > #include <linux/file.h> > > #include <linux/fs.h> > > +#include <linux/kernel.h> > > #include <linux/slab.h> > > #include <linux/namei.h> > > #include <linux/swap.h> > > @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp > > copy_clid(new, conf); > > gen_confirm(new, nn); > > } else /* case 4 (new client) or cases 2, 3 (client reboot): */ > > - ; > > + do_empty(); Urgh... This is just for documentation purposes anyway, so why not just turn it all into a comment by moving the 'else' into the comment field? i.e. } /* else case 4 (.... */ new->cl_minorversion = 0; > > gen_callback(new, setclid, rqstp); > > add_to_unconfirmed(new); > > -- > Chuck Lever > >
On 4/18/20 3:28 PM, Trond Myklebust wrote: > On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote: >>> On Apr 18, 2020, at 2:41 PM, Randy Dunlap <rdunlap@infradead.org> >>> wrote: >>> >>> Fix gcc empty-body warning when -Wextra is used: >>> >>> ../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty >>> body in an ‘else’ statement [-Wempty-body] >>> >>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >>> Cc: Linus Torvalds <torvalds@linux-foundation.org> >>> Cc: Andrew Morton <akpm@linux-foundation.org> >>> Cc: "J. Bruce Fields" <bfields@fieldses.org> >>> Cc: Chuck Lever <chuck.lever@oracle.com> >>> Cc: linux-nfs@vger.kernel.org >> >> I have a patch in my queue that addresses this particular warning, >> but your change works for me too. >> >> Acked-by: Chuck Lever <chuck.lever@oracle.com> >> >> Unless Bruce objects. >> >> >>> --- >>> fs/nfsd/nfs4state.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> --- linux-next-20200417.orig/fs/nfsd/nfs4state.c >>> +++ linux-next-20200417/fs/nfsd/nfs4state.c >>> @@ -34,6 +34,7 @@ >>> >>> #include <linux/file.h> >>> #include <linux/fs.h> >>> +#include <linux/kernel.h> >>> #include <linux/slab.h> >>> #include <linux/namei.h> >>> #include <linux/swap.h> >>> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp >>> copy_clid(new, conf); >>> gen_confirm(new, nn); >>> } else /* case 4 (new client) or cases 2, 3 (client reboot): */ >>> - ; >>> + do_empty(); > > Urgh... This is just for documentation purposes anyway, so why not just > turn it all into a comment by moving the 'else' into the comment field? > > i.e. > } /* else case 4 (.... */ > > new->cl_minorversion = 0; >>> gen_callback(new, setclid, rqstp); >>> add_to_unconfirmed(new); Like I said earlier, since Chuck has a patch that addresses this, let's just go with that. thanks.
> On Apr 18, 2020, at 6:32 PM, Randy Dunlap <rdunlap@infradead.org> wrote: > > On 4/18/20 3:28 PM, Trond Myklebust wrote: >> On Sat, 2020-04-18 at 14:45 -0400, Chuck Lever wrote: >>>> On Apr 18, 2020, at 2:41 PM, Randy Dunlap <rdunlap@infradead.org> >>>> wrote: >>>> >>>> Fix gcc empty-body warning when -Wextra is used: >>>> >>>> ../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty >>>> body in an ‘else’ statement [-Wempty-body] >>>> >>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org> >>>> Cc: Linus Torvalds <torvalds@linux-foundation.org> >>>> Cc: Andrew Morton <akpm@linux-foundation.org> >>>> Cc: "J. Bruce Fields" <bfields@fieldses.org> >>>> Cc: Chuck Lever <chuck.lever@oracle.com> >>>> Cc: linux-nfs@vger.kernel.org >>> >>> I have a patch in my queue that addresses this particular warning, >>> but your change works for me too. >>> >>> Acked-by: Chuck Lever <chuck.lever@oracle.com> >>> >>> Unless Bruce objects. >>> >>> >>>> --- >>>> fs/nfsd/nfs4state.c | 3 ++- >>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>> >>>> --- linux-next-20200417.orig/fs/nfsd/nfs4state.c >>>> +++ linux-next-20200417/fs/nfsd/nfs4state.c >>>> @@ -34,6 +34,7 @@ >>>> >>>> #include <linux/file.h> >>>> #include <linux/fs.h> >>>> +#include <linux/kernel.h> >>>> #include <linux/slab.h> >>>> #include <linux/namei.h> >>>> #include <linux/swap.h> >>>> @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp >>>> copy_clid(new, conf); >>>> gen_confirm(new, nn); >>>> } else /* case 4 (new client) or cases 2, 3 (client reboot): */ >>>> - ; >>>> + do_empty(); >> >> Urgh... This is just for documentation purposes anyway, so why not just >> turn it all into a comment by moving the 'else' into the comment field? >> >> i.e. >> } /* else case 4 (.... */ >> >> new->cl_minorversion = 0; >>>> gen_callback(new, setclid, rqstp); >>>> add_to_unconfirmed(new); > > Like I said earlier, since Chuck has a patch that addresses this, > let's just go with that. I'll post that patch for review as part of my NFSD for-5.8 patches. -- Chuck Lever
Hello! On 18.04.2020 21:41, Randy Dunlap wrote: > Fix gcc empty-body warning when -Wextra is used: > > ../fs/nfsd/nfs4state.c:3898:3: warning: suggest braces around empty body in an ‘else’ statement [-Wempty-body] > > Signed-off-by: Randy Dunlap <rdunlap@infradead.org> > Cc: Linus Torvalds <torvalds@linux-foundation.org> > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: "J. Bruce Fields" <bfields@fieldses.org> > Cc: Chuck Lever <chuck.lever@oracle.com> > Cc: linux-nfs@vger.kernel.org > --- > fs/nfsd/nfs4state.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > --- linux-next-20200417.orig/fs/nfsd/nfs4state.c > +++ linux-next-20200417/fs/nfsd/nfs4state.c [...] > @@ -3895,7 +3896,7 @@ nfsd4_setclientid(struct svc_rqst *rqstp > copy_clid(new, conf); > gen_confirm(new, nn); > } else /* case 4 (new client) or cases 2, 3 (client reboot): */ > - ; > + do_empty(); In this case explicit {} would probably have been better, as described in Documentation/process/coding-style.rst, clause (3). MBR, Sergei
--- 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);
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(-)