| Message ID | 20200917214545.199463-1-ndesaulniers@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v3] nfs: remove incorrect fallthrough label | expand |
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 >
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;
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/
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
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/ > >
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.
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;
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(-)