[Drbd-dev] [PATCH RESEND] drbd: avoid clang warning about pointless switch statement

[Drbd-dev] [PATCH RESEND] drbd: avoid clang warning about pointless switch statement

[Nathan Chancellor]

From: Arnd Bergmann <arnd@arndb.de>

The drbd code causes warnings that we cannot easily disable when building with clang:

In file included from drivers/block/drbd/drbd_debugfs.c:10:
In file included from drivers/block/drbd/drbd_int.h:48:
In file included from include/linux/drbd_genl_api.h:53:
In file included from include/linux/genl_magic_struct.h:237:
include/linux/drbd_genl.h:300:1: warning: no case matching constant switch condition '0'

There is nothing wrong with the code, and adding 'default:' labels
in the right place is enough to let clang shut up about the warning.

Fixes: ec2c35ac1ea2 ("drbd: prepare the transition from connector to genetlink")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

This was recently resent by Philipp Klocke but the attribution on it
was incorrect (had Arnd's signoff but no From: line) and linux-block
wasn't cc'd, who presumably should take this patch since this file was
introduced by commit ec2c35ac1ea2 ("drbd: prepare the transition from
connector to genetlink"). If that is the case, perhaps these files
should be added to the drbd MAINTAINERS entry since they were introduced
by drbd and have been the only consumers since; I'd be happy to send a
patch if others agree.

It was acked by Lars on https://lore.kernel.org/patchwork/patch/756723/
so I don't know if this patch should carry

Acked-by: Lars Ellenberg <lars.ellenberg@linbit.com>

or not. I'll leave that up to the maintainer to decide.

Thanks!
Nathan

 include/linux/genl_magic_struct.h | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
index 5972e4969197..c11f42179c63 100644
--- a/include/linux/genl_magic_struct.h
+++ b/include/linux/genl_magic_struct.h
@@ -191,6 +191,7 @@ static inline void ct_assert_unique_operations(void)
 {
 	switch (0) {
 #include GENL_MAGIC_INCLUDE_FILE
+	default:
 		;
 	}
 }
@@ -209,6 +210,7 @@ static inline void ct_assert_unique_top_level_attributes(void)
 {
 	switch (0) {
 #include GENL_MAGIC_INCLUDE_FILE
+	default:
 		;
 	}
 }
@@ -219,6 +221,7 @@ static inline void ct_assert_unique_ ## s_name ## _attributes(void)	\
 {									\
 	switch (0) {							\
 		s_fields						\
+		default:						\
 			;						\
 	}								\
 }
-- 
2.19.0

Re: [Drbd-dev] [PATCH RESEND] drbd: avoid clang warning about pointless switch statement

[Lars Ellenberg]

On Tue, Oct 02, 2018 at 10:14:48AM -0700, Nathan Chancellor wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> The drbd code causes warnings that we cannot easily disable when building with clang:
> 
> In file included from drivers/block/drbd/drbd_debugfs.c:10:
> In file included from drivers/block/drbd/drbd_int.h:48:
> In file included from include/linux/drbd_genl_api.h:53:
> In file included from include/linux/genl_magic_struct.h:237:
> include/linux/drbd_genl.h:300:1: warning: no case matching constant switch condition '0'
> 
> There is nothing wrong with the code, and adding 'default:' labels
> in the right place is enough to let clang shut up about the warning.

Actually, I think I'd prefer a "case 0:" instead of the "default:",
because that would assert that we don't use a 0 for op or field numbers.

As at least some people seem to care, I suggest we bundle up
this and the other clang warning you just resent the patch for
with whatever else we may come up with for the next merge window
and send a pull request to Jens then.
In case we forget, please send a friendly reminder.

Thanks,

    Lars

Re: [Drbd-dev] [PATCH RESEND] drbd: avoid clang warning about pointless switch statement

[Nathan Chancellor]

On Thu, Oct 04, 2018 at 09:18:54PM +0200, Lars Ellenberg wrote:
> On Tue, Oct 02, 2018 at 10:14:48AM -0700, Nathan Chancellor wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> > 
> > The drbd code causes warnings that we cannot easily disable when building with clang:
> > 
> > In file included from drivers/block/drbd/drbd_debugfs.c:10:
> > In file included from drivers/block/drbd/drbd_int.h:48:
> > In file included from include/linux/drbd_genl_api.h:53:
> > In file included from include/linux/genl_magic_struct.h:237:
> > include/linux/drbd_genl.h:300:1: warning: no case matching constant switch condition '0'
> > 
> > There is nothing wrong with the code, and adding 'default:' labels
> > in the right place is enough to let clang shut up about the warning.
> 
> Actually, I think I'd prefer a "case 0:" instead of the "default:",
> because that would assert that we don't use a 0 for op or field numbers.
> 
> As at least some people seem to care, I suggest we bundle up
> this and the other clang warning you just resent the patch for
> with whatever else we may come up with for the next merge window
> and send a pull request to Jens then.
> In case we forget, please send a friendly reminder.
> 
> Thanks,
> 
>     Lars
> 

Hi Lars,

I will go ahead and test that change shortly and send it over for
review.

Thanks for the quick reply!
Nathan

[Drbd-dev] [PATCH v2] drbd: Avoid Clang warning about pointless switch statment

[Nathan Chancellor]

There are several warnings from Clang about no case statement matching
the constant 0:

In file included from drivers/block/drbd/drbd_receiver.c:48:
In file included from drivers/block/drbd/drbd_int.h:48:
In file included from ./include/linux/drbd_genl_api.h:54:
In file included from ./include/linux/genl_magic_struct.h:236:
./include/linux/drbd_genl.h:321:1: warning: no case matching constant
switch condition '0'
GENL_struct(DRBD_NLA_HELPER, 24, drbd_helper_info,
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./include/linux/genl_magic_struct.h:220:10: note: expanded from macro
'GENL_struct'
        switch (0) {
                ^

Silence this warning by adding a 'case 0:' statement. Additionally,
adjust the alignment of the statements in the ct_assert_unique macro to
avoid a checkpatch warning.

This solution was originally sent by Arnd Bergmann with a default case
statement: https://lore.kernel.org/patchwork/patch/756723/

Link: https://github.com/ClangBuiltLinux/linux/issues/43
Suggested-by: Lars Ellenberg <lars.ellenberg@linbit.com>
Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
---

Since this is not the same as Arnd's patch, I took authorship for it
while leaving a link to the original patch in the commit message. If
this is not how it should have been done, please let me know.

 include/linux/genl_magic_struct.h | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
index 5972e4969197..eeae59d3ceb7 100644
--- a/include/linux/genl_magic_struct.h
+++ b/include/linux/genl_magic_struct.h
@@ -191,6 +191,7 @@ static inline void ct_assert_unique_operations(void)
 {
 	switch (0) {
 #include GENL_MAGIC_INCLUDE_FILE
+	case 0:
 		;
 	}
 }
@@ -209,6 +210,7 @@ static inline void ct_assert_unique_top_level_attributes(void)
 {
 	switch (0) {
 #include GENL_MAGIC_INCLUDE_FILE
+	case 0:
 		;
 	}
 }
@@ -218,7 +220,8 @@ static inline void ct_assert_unique_top_level_attributes(void)
 static inline void ct_assert_unique_ ## s_name ## _attributes(void)	\
 {									\
 	switch (0) {							\
-		s_fields						\
+	s_fields							\
+	case 0:								\
 			;						\
 	}								\
 }
-- 
2.19.0

Re: [Drbd-dev] [PATCH v2] drbd: Avoid Clang warning about pointless switch statment

[Jens Axboe]

On 12/17/18 10:24 AM, Nathan Chancellor wrote:
> On Thu, Oct 04, 2018 at 01:09:13PM -0700, Nathan Chancellor wrote:
>> There are several warnings from Clang about no case statement matching
>> the constant 0:
>>
>> In file included from drivers/block/drbd/drbd_receiver.c:48:
>> In file included from drivers/block/drbd/drbd_int.h:48:
>> In file included from ./include/linux/drbd_genl_api.h:54:
>> In file included from ./include/linux/genl_magic_struct.h:236:
>> ./include/linux/drbd_genl.h:321:1: warning: no case matching constant
>> switch condition '0'
>> GENL_struct(DRBD_NLA_HELPER, 24, drbd_helper_info,
>> ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>> ./include/linux/genl_magic_struct.h:220:10: note: expanded from macro
>> 'GENL_struct'
>>         switch (0) {
>>                 ^
>>
>> Silence this warning by adding a 'case 0:' statement. Additionally,
>> adjust the alignment of the statements in the ct_assert_unique macro to
>> avoid a checkpatch warning.
>>
>> This solution was originally sent by Arnd Bergmann with a default case
>> statement: https://lore.kernel.org/patchwork/patch/756723/
>>
>> Link: https://github.com/ClangBuiltLinux/linux/issues/43
>> Suggested-by: Lars Ellenberg <lars.ellenberg@linbit.com>
>> Signed-off-by: Nathan Chancellor <natechancellor@gmail.com>
>> ---
>>
>> Since this is not the same as Arnd's patch, I took authorship for it
>> while leaving a link to the original patch in the commit message. If
>> this is not how it should have been done, please let me know.
>>
>>  include/linux/genl_magic_struct.h | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/include/linux/genl_magic_struct.h b/include/linux/genl_magic_struct.h
>> index 5972e4969197..eeae59d3ceb7 100644
>> --- a/include/linux/genl_magic_struct.h
>> +++ b/include/linux/genl_magic_struct.h
>> @@ -191,6 +191,7 @@ static inline void ct_assert_unique_operations(void)
>>  {
>>  	switch (0) {
>>  #include GENL_MAGIC_INCLUDE_FILE
>> +	case 0:
>>  		;
>>  	}
>>  }
>> @@ -209,6 +210,7 @@ static inline void ct_assert_unique_top_level_attributes(void)
>>  {
>>  	switch (0) {
>>  #include GENL_MAGIC_INCLUDE_FILE
>> +	case 0:
>>  		;
>>  	}
>>  }
>> @@ -218,7 +220,8 @@ static inline void ct_assert_unique_top_level_attributes(void)
>>  static inline void ct_assert_unique_ ## s_name ## _attributes(void)	\
>>  {									\
>>  	switch (0) {							\
>> -		s_fields						\
>> +	s_fields							\
>> +	case 0:								\
>>  			;						\
>>  	}								\
>>  }
>> -- 
>> 2.19.0
>>
> 
> Hi Lars and Philipp,
> 
> Could you please make sure that this patch and the other one I sent make
> it into 4.21/5.0? I am not sure when you were planning on sending the
> pull request to Jens that you mentioned in the other thread but I've
> noticed most maintainers typically send their requests for the impending
> merge window around -rc7 or so and I wanted to make sure it was on your
> radar.

It needs to get here now, but drbd hasn't really sent anything in for
about a year, so I'm starting to doubt how maintained it is at this
point.

-- 
Jens Axboe

Re: [Drbd-dev] [PATCH v2] drbd: Avoid Clang warning about pointless switch statment

[Lars Ellenberg]

On Mon, Dec 17, 2018 at 10:29:38AM -0700, Jens Axboe wrote:
> > Hi Lars and Philipp,
> > 
> > Could you please make sure that this patch and the other one I sent make
> > it into 4.21/5.0? I am not sure when you were planning on sending the
> > pull request to Jens that you mentioned in the other thread but I've
> > noticed most maintainers typically send their requests for the impending
> > merge window around -rc7 or so and I wanted to make sure it was on your
> > radar.

I'm sorry.
From my point of view, "fixing the pointless switch" is just "pointless",
so it (again) fell through.  I get it that it is important to others,
and getting rid of Clang warnings is a good thing.  So sorry again.

> It needs to get here now, but drbd hasn't really sent anything in for
> about a year,

Last actual fix was 2018-06-25 64dafbc9530c drbd: fix access after free
so almost six month, yes.

> so I'm starting to doubt how maintained it is at this
> point.

Oh, it is maintained.  It is just happens to be "stable".
We don't add new features there,
and I'm currently not aware of any misbehavior.

I'll prepare a pull request containing this,
the other Clang warning patch from Nathan,
and possibly other small stuff that accumulated,
if I can find any, and send that out later today.

Thanks for the reminder,

    Lars

Re: [Drbd-dev] [PATCH v2] drbd: Avoid Clang warning about pointless switch statment

[Jens Axboe]

On 12/18/18 2:22 AM, Lars Ellenberg wrote:
> On Mon, Dec 17, 2018 at 10:29:38AM -0700, Jens Axboe wrote:
>>> Hi Lars and Philipp,
>>>
>>> Could you please make sure that this patch and the other one I sent make
>>> it into 4.21/5.0? I am not sure when you were planning on sending the
>>> pull request to Jens that you mentioned in the other thread but I've
>>> noticed most maintainers typically send their requests for the impending
>>> merge window around -rc7 or so and I wanted to make sure it was on your
>>> radar.
> 
> I'm sorry.
> From my point of view, "fixing the pointless switch" is just "pointless",
> so it (again) fell through.  I get it that it is important to others,
> and getting rid of Clang warnings is a good thing.  So sorry again.
> 
>> It needs to get here now, but drbd hasn't really sent anything in for
>> about a year,
> 
> Last actual fix was 2018-06-25 64dafbc9530c drbd: fix access after free
> so almost six month, yes.

It just dropped from its usual cadence of regular updates.

>> so I'm starting to doubt how maintained it is at this
>> point.
> 
> Oh, it is maintained.  It is just happens to be "stable".
> We don't add new features there,
> and I'm currently not aware of any misbehavior.

Well, I guess that's good news then :-)

> I'll prepare a pull request containing this,
> the other Clang warning patch from Nathan,
> and possibly other small stuff that accumulated,
> if I can find any, and send that out later today.
> 
> Thanks for the reminder,

Thank.

-- 
Jens Axboe