[PATCH] nfs: remove incorrect fallthrough label

[PATCH] nfs: remove incorrect fallthrough label

[Nick Desaulniers]

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

Re: [PATCH] nfs: remove incorrect fallthrough label

[Joe Perches]

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.

>  		}
> 

Re: [PATCH] nfs: remove incorrect fallthrough label

[Gustavo A. R. Silva]

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

Re: [PATCH] nfs: remove incorrect fallthrough label

[Gustavo A. R. Silva]

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

Re: [PATCH] nfs: remove incorrect fallthrough label

[Joe Perches]

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.

[PATCH v2] nfs: remove incorrect fallthrough label

[Nick Desaulniers]

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

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[Gustavo A. R. Silva]

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
> 

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[Joe Perches]

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?

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[Nathan Chancellor]

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
> 

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[Nick Desaulniers]

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

[PATCH v3] nfs: remove incorrect fallthrough label

[Nick Desaulniers]

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

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[Joe Perches]

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*:' *

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Nick Desaulniers]

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

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Joe Perches]

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;

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Joe Perches]

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/

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Gustavo A. R. Silva]

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

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Anna Schumaker]

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/
>
>

Re: [PATCH v3] nfs: remove incorrect fallthrough label

[Mark Brown]

[-- 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 --]

Re: [PATCH v2] nfs: remove incorrect fallthrough label

[linmiaohe]

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
>* __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: Miaohe Lin <linmiaohe@huawei.com>