| Message ID | 20200915225751.274531-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | nfs: remove incorrect fallthrough label | expand |
On Tue, 2020-09-15 at 15:57 -0700, Nick Desaulniers wrote: > There is no case after the default from which to fallthrough to. Clang > will error in this case (unhelpfully without context, see link below) > and GCC will with -Wswitch-unreachable. > > The previous commit should have just removed the comment. [] > diff --git a/fs/nfs/super.c b/fs/nfs/super.c [] > @@ -889,7 +889,6 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) > default: > if (rpcauth_get_gssinfo(flavor, &info) != 0) > continue; > - fallthrough; My preference would be to convert the fallthrough to a break here so if someone ever adds another label after default: for any reason, the code would still work as expected. > } >
On 9/15/20 18:29, Joe Perches wrote: > On Tue, 2020-09-15 at 15:57 -0700, Nick Desaulniers wrote: >> There is no case after the default from which to fallthrough to. Clang >> will error in this case (unhelpfully without context, see link below) >> and GCC will with -Wswitch-unreachable. >> >> The previous commit should have just removed the comment. > [] >> diff --git a/fs/nfs/super.c b/fs/nfs/super.c > [] >> @@ -889,7 +889,6 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) >> default: >> if (rpcauth_get_gssinfo(flavor, &info) != 0) >> continue; >> - fallthrough; > > My preference would be to convert the fallthrough > to a break here so if someone ever adds another > label after default: for any reason, the code would > still work as expected. I agree with Joe. Thanks -- Gustavo
On 9/15/20 18:51, Gustavo A. R. Silva wrote: > > > On 9/15/20 18:29, Joe Perches wrote: >> On Tue, 2020-09-15 at 15:57 -0700, Nick Desaulniers wrote: >>> There is no case after the default from which to fallthrough to. Clang >>> will error in this case (unhelpfully without context, see link below) >>> and GCC will with -Wswitch-unreachable. >>> >>> The previous commit should have just removed the comment. >> [] >>> diff --git a/fs/nfs/super.c b/fs/nfs/super.c >> [] >>> @@ -889,7 +889,6 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) >>> default: >>> if (rpcauth_get_gssinfo(flavor, &info) != 0) >>> continue; >>> - fallthrough; >> >> My preference would be to convert the fallthrough >> to a break here so if someone ever adds another >> label after default: for any reason, the code would >> still work as expected. > > I agree with Joe. Actually, this is part of the work I plan to do to enable -Wimplicit-fallthrough for Clang: audit every place where we could use a break instead of a fallthrough. I'm on vacation this week. So, I'll get back to this next week. Thanks -- Gustavo
On Tue, 2020-09-15 at 19:01 -0500, Gustavo A. R. Silva wrote: > > On 9/15/20 18:51, Gustavo A. R. Silva wrote: > > > > On 9/15/20 18:29, Joe Perches wrote: > > > On Tue, 2020-09-15 at 15:57 -0700, Nick Desaulniers wrote: > > > > There is no case after the default from which to fallthrough to. Clang > > > > will error in this case (unhelpfully without context, see link below) > > > > and GCC will with -Wswitch-unreachable. > > > > > > > > The previous commit should have just removed the comment. > > > [] > > > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > > > [] > > > > @@ -889,7 +889,6 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) > > > > default: > > > > if (rpcauth_get_gssinfo(flavor, &info) != 0) > > > > continue; > > > > - fallthrough; > > > > > > My preference would be to convert the fallthrough > > > to a break here so if someone ever adds another > > > label after default: for any reason, the code would > > > still work as expected. > > > > I agree with Joe. > > Actually, this is part of the work I plan to do to enable -Wimplicit-fallthrough > for Clang: audit every place where we could use a break instead of a fallthrough. > > I'm on vacation this week. So, I'll get back to this next week. Nice, thanks Gustavo. As part of that work, perhaps you could also find all the switch (<foo>) { [cases...] [code...]; break; default: [code...] (no break) } uawa where the last label/default block does _not_ have a break statement and add one too. Also see: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432 gcc does _not_ warn on switch (<foo>) { case BAR: [code]; (no fallthrough) case BAZ: break; } It might be good to add the appropriate fallthrough for those case blocks too.
diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d20326ee0475..7de4e0b03be0 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -889,7 +889,6 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) default: if (rpcauth_get_gssinfo(flavor, &info) != 0) continue; - fallthrough; } dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); ctx->selected_flavor = flavor;
There is no case after the default from which to fallthrough to. Clang will error in this case (unhelpfully without context, see link below) and GCC will with -Wswitch-unreachable. The previous commit should have just removed the comment. Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") Link: https://bugs.llvm.org/show_bug.cgi?id=47539 Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- fs/nfs/super.c | 1 - 1 file changed, 1 deletion(-)