* [PATCH] nfs: remove incorrect fallthrough label
@ 2020-09-15 22:57 Nick Desaulniers
2020-09-15 23:29 ` Joe Perches
0 siblings, 1 reply; 18+ messages in thread
From: Nick Desaulniers @ 2020-09-15 22:57 UTC (permalink / raw)
To: Trond Myklebust, Anna Schumaker
Cc: Gustavo A . R . Silva, Joe Perches, Nick Desaulniers,
Nathan Chancellor, Miaohe Lin, Hongxiang Lou, linux-nfs,
linux-kernel, clang-built-linux
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(-)
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;
--
2.28.0.618.gf4bc123cb7-goog
^ permalink raw reply related [flat|nested] 18+ messages in thread* Re: [PATCH] nfs: remove incorrect fallthrough label 2020-09-15 22:57 [PATCH] nfs: remove incorrect fallthrough label Nick Desaulniers @ 2020-09-15 23:29 ` Joe Perches 2020-09-15 23:51 ` Gustavo A. R. Silva 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers 0 siblings, 2 replies; 18+ messages in thread From: Joe Perches @ 2020-09-15 23:29 UTC (permalink / raw) To: Nick Desaulniers, Trond Myklebust, Anna Schumaker Cc: Gustavo A . R . Silva, Nathan Chancellor, Miaohe Lin, Hongxiang Lou, linux-nfs, linux-kernel, clang-built-linux 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. > } > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nfs: remove incorrect fallthrough label 2020-09-15 23:29 ` Joe Perches @ 2020-09-15 23:51 ` Gustavo A. R. Silva 2020-09-16 0:01 ` Gustavo A. R. Silva 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers 1 sibling, 1 reply; 18+ messages in thread From: Gustavo A. R. Silva @ 2020-09-15 23:51 UTC (permalink / raw) To: Joe Perches, Nick Desaulniers, Trond Myklebust, Anna Schumaker Cc: Nathan Chancellor, Miaohe Lin, Hongxiang Lou, linux-nfs, linux-kernel, clang-built-linux 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nfs: remove incorrect fallthrough label 2020-09-15 23:51 ` Gustavo A. R. Silva @ 2020-09-16 0:01 ` Gustavo A. R. Silva 2020-09-16 0:33 ` Joe Perches 0 siblings, 1 reply; 18+ messages in thread From: Gustavo A. R. Silva @ 2020-09-16 0:01 UTC (permalink / raw) To: Joe Perches, Nick Desaulniers, Trond Myklebust, Anna Schumaker Cc: Nathan Chancellor, Miaohe Lin, Hongxiang Lou, linux-nfs, linux-kernel, clang-built-linux 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 ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH] nfs: remove incorrect fallthrough label 2020-09-16 0:01 ` Gustavo A. R. Silva @ 2020-09-16 0:33 ` Joe Perches 0 siblings, 0 replies; 18+ messages in thread From: Joe Perches @ 2020-09-16 0:33 UTC (permalink / raw) To: Gustavo A. R. Silva, Nick Desaulniers, Trond Myklebust, Anna Schumaker Cc: Nathan Chancellor, Miaohe Lin, Hongxiang Lou, linux-nfs, linux-kernel, clang-built-linux 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. ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-15 23:29 ` Joe Perches 2020-09-15 23:51 ` Gustavo A. R. Silva @ 2020-09-16 20:02 ` Nick Desaulniers 2020-09-16 20:18 ` Gustavo A. R. Silva ` (2 more replies) 1 sibling, 3 replies; 18+ messages in thread From: Nick Desaulniers @ 2020-09-16 20:02 UTC (permalink / raw) To: Trond Myklebust, Anna Schumaker Cc: Gustavo A . R . Silva, Joe Perches, Nick Desaulniers, Nathan Chancellor, Hongxiang Lou, Miaohe Lin, linux-nfs, linux-kernel, clang-built-linux 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 replaced the comment with a break statement. If we consider implicit fallthrough to be a design mistake of C, then all case statements should be terminated with one of the following statements: * break * continue * return * __attribute__(__fallthrough__) * goto (plz no) * (call of function with __attribute__(__noreturn__)) Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") Link: https://bugs.llvm.org/show_bug.cgi?id=47539 Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v2: * add break rather than no terminating statement as per Joe. * add Joe's suggested by tag. * add blurb about acceptable terminal statements. fs/nfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d20326ee0475..eb2401079b04 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) default: if (rpcauth_get_gssinfo(flavor, &info) != 0) continue; - fallthrough; + break; } dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); ctx->selected_flavor = flavor; -- 2.28.0.618.gf4bc123cb7-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers @ 2020-09-16 20:18 ` Gustavo A. R. Silva 2020-09-16 20:19 ` Joe Perches 2020-09-17 4:35 ` [PATCH v2] " Nathan Chancellor 2 siblings, 0 replies; 18+ messages in thread From: Gustavo A. R. Silva @ 2020-09-16 20:18 UTC (permalink / raw) To: Nick Desaulniers Cc: Trond Myklebust, Anna Schumaker, Gustavo A . R . Silva, Joe Perches, Nathan Chancellor, Hongxiang Lou, Miaohe Lin, linux-nfs, linux-kernel, clang-built-linux On Wed, Sep 16, 2020 at 01:02:55PM -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 replaced the comment with a break > statement. > > If we consider implicit fallthrough to be a design mistake of C, then > all case statements should be terminated with one of the following > statements: > * break > * continue > * return > * __attribute__(__fallthrough__) > * goto (plz no) > * (call of function with __attribute__(__noreturn__)) > > Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") > Link: https://bugs.llvm.org/show_bug.cgi?id=47539 > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Thanks -- Gustavo > --- > Changes v2: > * add break rather than no terminating statement as per Joe. > * add Joe's suggested by tag. > * add blurb about acceptable terminal statements. > > fs/nfs/super.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index d20326ee0475..eb2401079b04 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) > default: > if (rpcauth_get_gssinfo(flavor, &info) != 0) > continue; > - fallthrough; > + break; > } > dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); > ctx->selected_flavor = flavor; > -- > 2.28.0.618.gf4bc123cb7-goog > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers 2020-09-16 20:18 ` Gustavo A. R. Silva @ 2020-09-16 20:19 ` Joe Perches 2020-09-17 21:41 ` Nick Desaulniers 2020-09-17 21:45 ` [PATCH v3] " Nick Desaulniers 2020-09-17 4:35 ` [PATCH v2] " Nathan Chancellor 2 siblings, 2 replies; 18+ messages in thread From: Joe Perches @ 2020-09-16 20:19 UTC (permalink / raw) To: Nick Desaulniers, Trond Myklebust, Anna Schumaker Cc: Gustavo A . R . Silva, Nathan Chancellor, Hongxiang Lou, Miaohe Lin, linux-nfs, linux-kernel, clang-built-linux On Wed, 2020-09-16 at 13:02 -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 replaced the comment with a break > statement. > If we consider implicit fallthrough to be a design mistake of C, then > all case statements should be terminated with one of the following > statements: > > * break > * continue > * return > * __attribute__(__fallthrough__) Just fallthrough. __attribute__((__fallthrough__) is only used once in code for the #define. And maybe add see: Documentation/process/deprecated.rst > * goto (plz no) goto is a valid style inside a switch/case label block. There are more than 1500 of these goto <label> uses in the kernel so the 'please no' here doesn't seem reasonable. > * (call of function with __attribute__(__noreturn__)) I guess panic counts. I count 11 of those. Are there any other uses of functions with __noreturn in switch/case label blocks? ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-16 20:19 ` Joe Perches @ 2020-09-17 21:41 ` Nick Desaulniers 2020-09-18 1:36 ` Joe Perches 2020-09-17 21:45 ` [PATCH v3] " Nick Desaulniers 1 sibling, 1 reply; 18+ messages in thread From: Nick Desaulniers @ 2020-09-17 21:41 UTC (permalink / raw) To: Joe Perches Cc: Trond Myklebust, Anna Schumaker, Gustavo A . R . Silva, Nathan Chancellor, Hongxiang Lou, Miaohe Lin, linux-nfs, LKML, clang-built-linux On Wed, Sep 16, 2020 at 1:19 PM Joe Perches <joe@perches.com> wrote: > > On Wed, 2020-09-16 at 13:02 -0700, Nick Desaulniers wrote: > > * (call of function with __attribute__(__noreturn__)) > > I guess panic counts. I count 11 of those. > > Are there any other uses of functions with __noreturn > in switch/case label blocks? If you look at global_noreturns in tools/objtool/check.c: 145 static const char * const global_noreturns[] = { 146 "__stack_chk_fail", 147 "panic", 148 "do_exit", 149 "do_task_dead", 150 "__module_put_and_exit", 151 "complete_and_exit", 152 "__reiserfs_panic", 153 "lbug_with_loc", 154 "fortify_panic", 155 "usercopy_abort", 156 "machine_real_restart", 157 "rewind_stack_do_exit", 158 "kunit_try_catch_throw", 159 }; Whether they occur or not at the position you ask; I haven't checked. -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-17 21:41 ` Nick Desaulniers @ 2020-09-18 1:36 ` Joe Perches 0 siblings, 0 replies; 18+ messages in thread From: Joe Perches @ 2020-09-18 1:36 UTC (permalink / raw) To: Nick Desaulniers Cc: Trond Myklebust, Anna Schumaker, Gustavo A . R . Silva, Nathan Chancellor, Hongxiang Lou, Miaohe Lin, linux-nfs, LKML, clang-built-linux On Thu, 2020-09-17 at 14:41 -0700, Nick Desaulniers wrote: > On Wed, Sep 16, 2020 at 1:19 PM Joe Perches <joe@perches.com> wrote: > > On Wed, 2020-09-16 at 13:02 -0700, Nick Desaulniers wrote: > > > * (call of function with __attribute__(__noreturn__)) > > > > I guess panic counts. I count 11 of those. > > > > Are there any other uses of functions with __noreturn > > in switch/case label blocks? > > If you look at global_noreturns in tools/objtool/check.c: > 145 static const char * const global_noreturns[] = { > 146 "__stack_chk_fail", > 147 "panic", > 148 "do_exit", > 149 "do_task_dead", > 150 "__module_put_and_exit", > 151 "complete_and_exit", > 152 "__reiserfs_panic", > 153 "lbug_with_loc", > 154 "fortify_panic", > 155 "usercopy_abort", > 156 "machine_real_restart", > 157 "rewind_stack_do_exit", > 158 "kunit_try_catch_throw", > 159 }; > > Whether they occur or not at the position you ask; I haven't checked. Just fyi: Other than the 11 instances of panic, I found only a single use of any other function above in a switch/case: drivers/pnp/pnpbios/core.c:163: complete_and_exit(&unload_sem, 0); case PNP_SYSTEM_NOT_DOCKED: Found with: $ grep-2.5.4 -rP --include=*.[ch] -n '\b(?:__stack_chk_fail|panic|do_exit|do_task_dead|__module_put_and_exit|complete_and_exit|__reiserfs_panic|lbug_with_loc|fortify_panic|usercopy_abort|machine_real_restart|rewind_stack_do_exit|kunit_try_catch_throw)\s*(?:\([^\)]*\))?\s*;\s*(case\s+\w+|default)\s*:' * ^ permalink raw reply [flat|nested] 18+ messages in thread
* [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-16 20:19 ` Joe Perches 2020-09-17 21:41 ` Nick Desaulniers @ 2020-09-17 21:45 ` Nick Desaulniers 2020-09-24 17:19 ` Nick Desaulniers 1 sibling, 1 reply; 18+ messages in thread From: Nick Desaulniers @ 2020-09-17 21:45 UTC (permalink / raw) To: Trond Myklebust, Anna Schumaker Cc: Gustavo A . R . Silva, Joe Perches, Nick Desaulniers, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, linux-nfs, linux-kernel, clang-built-linux 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 replaced the comment with a break statement. If we consider implicit fallthrough to be a design mistake of C, then all case statements should be terminated with one of the following statements: * break * continue * return * fallthrough * goto * (call of function with __attribute__(__noreturn__)) Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") Link: https://bugs.llvm.org/show_bug.cgi?id=47539 Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v3: * update the commit message as per Joe. * collect tags. Changes v2: * add break rather than no terminating statement as per Joe. * add Joe's suggested by tag. * add blurb about acceptable terminal statements. fs/nfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d20326ee0475..eb2401079b04 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) default: if (rpcauth_get_gssinfo(flavor, &info) != 0) continue; - fallthrough; + break; } dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); ctx->selected_flavor = flavor; -- 2.28.0.681.g6f77f65b4e-goog ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-17 21:45 ` [PATCH v3] " Nick Desaulniers @ 2020-09-24 17:19 ` Nick Desaulniers 2020-09-24 17:40 ` Joe Perches 2020-09-24 18:08 ` Gustavo A. R. Silva 0 siblings, 2 replies; 18+ messages in thread From: Nick Desaulniers @ 2020-09-24 17:19 UTC (permalink / raw) To: Trond Myklebust, Anna Schumaker Cc: Gustavo A . R . Silva, Joe Perches, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, linux-nfs, LKML, clang-built-linux, Andrew Morton, Mark Brown Hello maintainers, Would you mind please picking up this patch? KernelCI has been erroring for over a week without it. On Thu, Sep 17, 2020 at 2:45 PM Nick Desaulniers <ndesaulniers@google.com> 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 replaced the comment with a break > statement. > > If we consider implicit fallthrough to be a design mistake of C, then > all case statements should be terminated with one of the following > statements: > * break > * continue > * return > * fallthrough > * goto > * (call of function with __attribute__(__noreturn__)) > > Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") > Link: https://bugs.llvm.org/show_bug.cgi?id=47539 > Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> > Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> > Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> > --- > Changes v3: > * update the commit message as per Joe. > * collect tags. > > Changes v2: > * add break rather than no terminating statement as per Joe. > * add Joe's suggested by tag. > * add blurb about acceptable terminal statements. > > fs/nfs/super.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index d20326ee0475..eb2401079b04 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) > default: > if (rpcauth_get_gssinfo(flavor, &info) != 0) > continue; > - fallthrough; > + break; > } > dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); > ctx->selected_flavor = flavor; > -- > 2.28.0.681.g6f77f65b4e-goog > -- Thanks, ~Nick Desaulniers ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-24 17:19 ` Nick Desaulniers @ 2020-09-24 17:40 ` Joe Perches 2020-09-24 18:07 ` Joe Perches 2020-09-24 18:08 ` Gustavo A. R. Silva 1 sibling, 1 reply; 18+ messages in thread From: Joe Perches @ 2020-09-24 17:40 UTC (permalink / raw) To: Nick Desaulniers, Trond Myklebust, Anna Schumaker, Linus Torvalds Cc: Gustavo A . R . Silva, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, linux-nfs, LKML, clang-built-linux, Andrew Morton, Mark Brown On Thu, 2020-09-24 at 10:19 -0700, Nick Desaulniers wrote: > Hello maintainers, > Would you mind please picking up this patch? KernelCI has been > erroring for over a week without it. As it's trivial and necessary, why not go through Linus directly? https://lore.kernel.org/patchwork/patch/1307549/ From: Nick Desaulniers <ndesaulniers@google.com> 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 replaced the comment with a break statement. If we consider implicit fallthrough to be a design mistake of C, then all case statements should be terminated with one of the following statements: * break * continue * return * fallthrough * goto * (call of function with __attribute__(__noreturn__)) Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") Link: https://bugs.llvm.org/show_bug.cgi?id=47539 Acked-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@kernel.org> Reviewed-by: Miaohe Lin <linmiaohe@huawei.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> Suggested-by: Joe Perches <joe@perches.com> Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> --- Changes v3: * update the commit message as per Joe. * collect tags. Changes v2: * add break rather than no terminating statement as per Joe. * add Joe's suggested by tag. * add blurb about acceptable terminal statements. fs/nfs/super.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/nfs/super.c b/fs/nfs/super.c index d20326ee0475..eb2401079b04 100644 --- a/fs/nfs/super.c +++ b/fs/nfs/super.c @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) default: if (rpcauth_get_gssinfo(flavor, &info) != 0) continue; - fallthrough; + break; } dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); ctx->selected_flavor = flavor; ^ permalink raw reply related [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-24 17:40 ` Joe Perches @ 2020-09-24 18:07 ` Joe Perches 2020-09-24 18:11 ` Anna Schumaker 0 siblings, 1 reply; 18+ messages in thread From: Joe Perches @ 2020-09-24 18:07 UTC (permalink / raw) To: Nick Desaulniers, Trond Myklebust, Anna Schumaker, Linus Torvalds Cc: Gustavo A . R . Silva, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, linux-nfs, LKML, clang-built-linux, Andrew Morton, Mark Brown On Thu, 2020-09-24 at 10:40 -0700, Joe Perches wrote: > On Thu, 2020-09-24 at 10:19 -0700, Nick Desaulniers wrote: > > Hello maintainers, > > Would you mind please picking up this patch? KernelCI has been > > erroring for over a week without it. > > As it's trivial and necessary, why not go through Linus directly? [] > Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") Real reason why not: the commit to be fixed is not in Linus' tree. > https://lore.kernel.org/patchwork/patch/1307549/ ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-24 18:07 ` Joe Perches @ 2020-09-24 18:11 ` Anna Schumaker 2020-09-25 11:27 ` Mark Brown 0 siblings, 1 reply; 18+ messages in thread From: Anna Schumaker @ 2020-09-24 18:11 UTC (permalink / raw) To: Joe Perches Cc: Nick Desaulniers, Trond Myklebust, Linus Torvalds, Gustavo A . R . Silva, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, Linux NFS Mailing List, LKML, clang-built-linux, Andrew Morton, Mark Brown On Thu, Sep 24, 2020 at 2:08 PM Joe Perches <joe@perches.com> wrote: > > On Thu, 2020-09-24 at 10:40 -0700, Joe Perches wrote: > > On Thu, 2020-09-24 at 10:19 -0700, Nick Desaulniers wrote: > > > Hello maintainers, > > > Would you mind please picking up this patch? KernelCI has been > > > erroring for over a week without it. > > > > As it's trivial and necessary, why not go through Linus directly? > [] > > Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") > > Real reason why not: > I'm planning to take this patch through the NFS tree for 5.10 (along with the patch that apparently causes the problem). I didn't think it was urgent so I haven't gotten around to pushing it out yet, but I'll do so in the next few hours. Anna > the commit to be fixed is not in Linus' tree. > > > https://lore.kernel.org/patchwork/patch/1307549/ > > ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-24 18:11 ` Anna Schumaker @ 2020-09-25 11:27 ` Mark Brown 0 siblings, 0 replies; 18+ messages in thread From: Mark Brown @ 2020-09-25 11:27 UTC (permalink / raw) To: Anna Schumaker Cc: Joe Perches, Nick Desaulniers, Trond Myklebust, Linus Torvalds, Gustavo A . R . Silva, Gustavo A . R . Silva, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, Linux NFS Mailing List, LKML, clang-built-linux, Andrew Morton [-- Attachment #1: Type: text/plain, Size: 613 bytes --] On Thu, Sep 24, 2020 at 02:11:59PM -0400, Anna Schumaker wrote: > On Thu, Sep 24, 2020 at 2:08 PM Joe Perches <joe@perches.com> wrote: > > Real reason why not: > I'm planning to take this patch through the NFS tree for 5.10 (along > with the patch that apparently causes the problem). I didn't think it > was urgent so I haven't gotten around to pushing it out yet, but I'll > do so in the next few hours. FWIW NFS is quite widely used by CI systems so any build breaks with it in -next have a pretty big knock on effect on testing, even beyond the distruption people working on the build test side of things. [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 488 bytes --] ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v3] nfs: remove incorrect fallthrough label 2020-09-24 17:19 ` Nick Desaulniers 2020-09-24 17:40 ` Joe Perches @ 2020-09-24 18:08 ` Gustavo A. R. Silva 1 sibling, 0 replies; 18+ messages in thread From: Gustavo A. R. Silva @ 2020-09-24 18:08 UTC (permalink / raw) To: Nick Desaulniers Cc: Trond Myklebust, Anna Schumaker, Gustavo A . R . Silva, Joe Perches, Miaohe Lin, Nathan Chancellor, Hongxiang Lou, linux-nfs, LKML, clang-built-linux, Andrew Morton, Mark Brown On Thu, Sep 24, 2020 at 10:19:08AM -0700, Nick Desaulniers wrote: > Hello maintainers, > Would you mind please picking up this patch? KernelCI has been > erroring for over a week without it. > I can add this to my -next tree and queue it up for the next merge window. Thanks -- Gustavo ^ permalink raw reply [flat|nested] 18+ messages in thread
* Re: [PATCH v2] nfs: remove incorrect fallthrough label 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers 2020-09-16 20:18 ` Gustavo A. R. Silva 2020-09-16 20:19 ` Joe Perches @ 2020-09-17 4:35 ` Nathan Chancellor 2 siblings, 0 replies; 18+ messages in thread From: Nathan Chancellor @ 2020-09-17 4:35 UTC (permalink / raw) To: Nick Desaulniers Cc: Trond Myklebust, Anna Schumaker, Gustavo A . R . Silva, Joe Perches, Hongxiang Lou, Miaohe Lin, linux-nfs, linux-kernel, clang-built-linux On Wed, Sep 16, 2020 at 01:02:55PM -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 replaced the comment with a break > statement. > > If we consider implicit fallthrough to be a design mistake of C, then > all case statements should be terminated with one of the following > statements: > * break > * continue > * return > * __attribute__(__fallthrough__) > * goto (plz no) > * (call of function with __attribute__(__noreturn__)) > > Fixes: 2a1390c95a69 ("nfs: Convert to use the preferred fallthrough macro") > Link: https://bugs.llvm.org/show_bug.cgi?id=47539 > Suggested-by: Joe Perches <joe@perches.com> > Signed-off-by: Nick Desaulniers <ndesaulniers@google.com> Reviewed-by: Nathan Chancellor <natechancellor@gmail.com> > --- > Changes v2: > * add break rather than no terminating statement as per Joe. > * add Joe's suggested by tag. > * add blurb about acceptable terminal statements. > > fs/nfs/super.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/nfs/super.c b/fs/nfs/super.c > index d20326ee0475..eb2401079b04 100644 > --- a/fs/nfs/super.c > +++ b/fs/nfs/super.c > @@ -889,7 +889,7 @@ static struct nfs_server *nfs_try_mount_request(struct fs_context *fc) > default: > if (rpcauth_get_gssinfo(flavor, &info) != 0) > continue; > - fallthrough; > + break; > } > dfprintk(MOUNT, "NFS: attempting to use auth flavor %u\n", flavor); > ctx->selected_flavor = flavor; > -- > 2.28.0.618.gf4bc123cb7-goog > ^ permalink raw reply [flat|nested] 18+ messages in thread
end of thread, other threads:[~2020-09-25 11:28 UTC | newest] Thread overview: 18+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2020-09-15 22:57 [PATCH] nfs: remove incorrect fallthrough label Nick Desaulniers 2020-09-15 23:29 ` Joe Perches 2020-09-15 23:51 ` Gustavo A. R. Silva 2020-09-16 0:01 ` Gustavo A. R. Silva 2020-09-16 0:33 ` Joe Perches 2020-09-16 20:02 ` [PATCH v2] " Nick Desaulniers 2020-09-16 20:18 ` Gustavo A. R. Silva 2020-09-16 20:19 ` Joe Perches 2020-09-17 21:41 ` Nick Desaulniers 2020-09-18 1:36 ` Joe Perches 2020-09-17 21:45 ` [PATCH v3] " Nick Desaulniers 2020-09-24 17:19 ` Nick Desaulniers 2020-09-24 17:40 ` Joe Perches 2020-09-24 18:07 ` Joe Perches 2020-09-24 18:11 ` Anna Schumaker 2020-09-25 11:27 ` Mark Brown 2020-09-24 18:08 ` Gustavo A. R. Silva 2020-09-17 4:35 ` [PATCH v2] " Nathan Chancellor
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).