linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
@ 2018-08-13 22:09 Arnd Bergmann
  2018-08-13 23:57 ` Masahiro Yamada
  2018-08-14 23:11 ` Andrew Morton
  0 siblings, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2018-08-13 22:09 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Arnd Bergmann, Masahiro Yamada, Johannes Berg, Jakub Kicinski,
	Andy Shevchenko, Al Viro, linux-kernel

Passing an enum into FIELD_GET() produces a long but harmless warning on
newer compilers:

                 from include/linux/linkage.h:7,
                 from include/linux/kernel.h:7,
                 from include/linux/skbuff.h:17,
                 from include/linux/if_ether.h:23,
                 from include/linux/etherdevice.h:25,
                 from drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:63:
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c: In function 'iwl_mvm_rx_mpdu_mq':
include/linux/bitfield.h:56:20: error: enum constant in boolean context [-Werror=int-in-bool-context]
   BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \
                    ^
...
include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
   __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
   ^~~~~~~~~~~~~~~~
drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
    le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,

The problem here is that the caller has no idea how the macro gets
expanding, leading to a false-positive. It can be trivially avoided
by doing a comparison against zero.

This only recently started appearing as the iwlwifi driver was patched
to use FIELD_GET.

Fixes: 514c30696fbc ("iwlwifi: add support for IEEE802.11ax")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 include/linux/bitfield.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
index 65a6981eef7b..3f1ef4450a7c 100644
--- a/include/linux/bitfield.h
+++ b/include/linux/bitfield.h
@@ -53,7 +53,7 @@
 	({								\
 		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
 				 _pfx "mask is not constant");		\
-		BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");	\
+		BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");	\
 		BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?		\
 				 ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \
 				 _pfx "value too large for the field"); \
-- 
2.18.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-13 22:09 [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning Arnd Bergmann
@ 2018-08-13 23:57 ` Masahiro Yamada
  2018-08-14  7:56   ` Johannes Berg
  2018-08-14 23:11 ` Andrew Morton
  1 sibling, 1 reply; 11+ messages in thread
From: Masahiro Yamada @ 2018-08-13 23:57 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Andrew Morton, Johannes Berg, Jakub Kicinski, Andy Shevchenko,
	Al Viro, Linux Kernel Mailing List

2018-08-14 7:09 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> Passing an enum into FIELD_GET() produces a long but harmless warning on
> newer compilers:
>
>                  from include/linux/linkage.h:7,
>                  from include/linux/kernel.h:7,
>                  from include/linux/skbuff.h:17,
>                  from include/linux/if_ether.h:23,
>                  from include/linux/etherdevice.h:25,
>                  from drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:63:
> drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c: In function 'iwl_mvm_rx_mpdu_mq':
> include/linux/bitfield.h:56:20: error: enum constant in boolean context [-Werror=int-in-bool-context]
>    BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \
>                     ^
> ...
> include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
>    __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
>    ^~~~~~~~~~~~~~~~
> drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
>     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,


How about fixing the root cause
in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?


#define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL


enum iwl_rx_he_phy looks really strange.



Passing enum to FIELD_GET is odd,
so I prefer keeping this warned.



> The problem here is that the caller has no idea how the macro gets
> expanding, leading to a false-positive. It can be trivially avoided
> by doing a comparison against zero.
>
> This only recently started appearing as the iwlwifi driver was patched
> to use FIELD_GET.
>
> Fixes: 514c30696fbc ("iwlwifi: add support for IEEE802.11ax")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/bitfield.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 65a6981eef7b..3f1ef4450a7c 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -53,7 +53,7 @@
>         ({                                                              \
>                 BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),          \
>                                  _pfx "mask is not constant");          \
> -               BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");        \
> +               BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");    \
>                 BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?           \
>                                  ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \
>                                  _pfx "value too large for the field"); \
> --
> 2.18.0
>



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-13 23:57 ` Masahiro Yamada
@ 2018-08-14  7:56   ` Johannes Berg
  2018-08-14  8:43     ` Arnd Bergmann
                       ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Johannes Berg @ 2018-08-14  7:56 UTC (permalink / raw)
  To: Masahiro Yamada, Arnd Bergmann
  Cc: Andrew Morton, Jakub Kicinski, Andy Shevchenko, Al Viro,
	Linux Kernel Mailing List

On Tue, 2018-08-14 at 08:57 +0900, Masahiro Yamada wrote:
> 2018-08-14 7:09 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
> > Passing an enum into FIELD_GET() produces a long but harmless warning on
> > newer compilers:
> > 
> >                  from include/linux/linkage.h:7,
> >                  from include/linux/kernel.h:7,
> >                  from include/linux/skbuff.h:17,
> >                  from include/linux/if_ether.h:23,
> >                  from include/linux/etherdevice.h:25,
> >                  from drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:63:
> > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c: In function 'iwl_mvm_rx_mpdu_mq':
> > include/linux/bitfield.h:56:20: error: enum constant in boolean context [-Werror=int-in-bool-context]
> >    BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \
> >                     ^
> > ...
> > include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
> >    __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
> >    ^~~~~~~~~~~~~~~~
> > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
> >     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,
> 
> 
> How about fixing the root cause
> in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?
> 
> 
> #define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL
> 
> 
> enum iwl_rx_he_phy looks really strange.

Why? I don't think this is a problem, the enum is used here to get
constants so that we can also have documentation for them. That's a
common and accepted technique.


> Passing enum to FIELD_GET is odd,
> so I prefer keeping this warned.

What for?

I think we should go with Arend's patch, and I hope Andrew will pick it
up, but otherwise I guess we can also put it through any other tree.

johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14  7:56   ` Johannes Berg
@ 2018-08-14  8:43     ` Arnd Bergmann
  2018-08-14  9:31     ` Masahiro Yamada
  2018-08-14 10:06     ` David Laight
  2 siblings, 0 replies; 11+ messages in thread
From: Arnd Bergmann @ 2018-08-14  8:43 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Masahiro Yamada, Andrew Morton, Jakub Kicinski, Andy Shevchenko,
	Al Viro, Linux Kernel Mailing List

On Tue, Aug 14, 2018 at 9:57 AM Johannes Berg <johannes@sipsolutions.net> wrote:
> On Tue, 2018-08-14 at 08:57 +0900, Masahiro Yamada wrote:
> > 2018-08-14 7:09 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:

> > > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
> > >     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,
> >
> >
> > How about fixing the root cause
> > in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?
> >
> >
> > #define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL
> >
> >
> > enum iwl_rx_he_phy looks really strange.
>
> Why? I don't think this is a problem, the enum is used here to get
> constants so that we can also have documentation for them. That's a
> common and accepted technique.

I think using #define is more common overall, but I also prefer using enum
for this in my own code.

   Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14  7:56   ` Johannes Berg
  2018-08-14  8:43     ` Arnd Bergmann
@ 2018-08-14  9:31     ` Masahiro Yamada
  2018-08-14 10:06     ` David Laight
  2 siblings, 0 replies; 11+ messages in thread
From: Masahiro Yamada @ 2018-08-14  9:31 UTC (permalink / raw)
  To: Johannes Berg
  Cc: Arnd Bergmann, Andrew Morton, Jakub Kicinski, Andy Shevchenko,
	Al Viro, Linux Kernel Mailing List

2018-08-14 16:56 GMT+09:00 Johannes Berg <johannes@sipsolutions.net>:
> On Tue, 2018-08-14 at 08:57 +0900, Masahiro Yamada wrote:
>> 2018-08-14 7:09 GMT+09:00 Arnd Bergmann <arnd@arndb.de>:
>> > Passing an enum into FIELD_GET() produces a long but harmless warning on
>> > newer compilers:
>> >
>> >                  from include/linux/linkage.h:7,
>> >                  from include/linux/kernel.h:7,
>> >                  from include/linux/skbuff.h:17,
>> >                  from include/linux/if_ether.h:23,
>> >                  from include/linux/etherdevice.h:25,
>> >                  from drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:63:
>> > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c: In function 'iwl_mvm_rx_mpdu_mq':
>> > include/linux/bitfield.h:56:20: error: enum constant in boolean context [-Werror=int-in-bool-context]
>> >    BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \
>> >                     ^
>> > ...
>> > include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
>> >    __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
>> >    ^~~~~~~~~~~~~~~~
>> > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
>> >     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,
>>
>>
>> How about fixing the root cause
>> in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?
>>
>>
>> #define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL
>>
>>
>> enum iwl_rx_he_phy looks really strange.
>
> Why? I don't think this is a problem, the enum is used here to get
> constants so that we can also have documentation for them. That's a
> common and accepted technique.


I do not see any variable declared as 'enum iwl_rx_he_phy'.

This is not legitimate usage of enum.


The mask macros must have a particular value,
hence

#define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL

is a straightforward way.


>
>> Passing enum to FIELD_GET is odd,
>> so I prefer keeping this warned.
>
> What for?


If you pass enum to FIELD_GET,
it is very like to be _abuse_ of enum.




> I think we should go with Arend's patch, and I hope Andrew will pick it
> up, but otherwise I guess we can also put it through any other tree.
>
> johannes



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14  7:56   ` Johannes Berg
  2018-08-14  8:43     ` Arnd Bergmann
  2018-08-14  9:31     ` Masahiro Yamada
@ 2018-08-14 10:06     ` David Laight
  2018-08-14 11:08       ` Arnd Bergmann
  2 siblings, 1 reply; 11+ messages in thread
From: David Laight @ 2018-08-14 10:06 UTC (permalink / raw)
  To: 'Johannes Berg', Masahiro Yamada, Arnd Bergmann
  Cc: Andrew Morton, Jakub Kicinski, Andy Shevchenko, Al Viro,
	Linux Kernel Mailing List

From: Johannes Berg
> Sent: 14 August 2018 08:57
...
> > How about fixing the root cause
> > in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?
> >
> >
> > #define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL
> >
> >
> > enum iwl_rx_he_phy looks really strange.
> 
> Why? I don't think this is a problem, the enum is used here to get
> constants so that we can also have documentation for them. That's a
> common and accepted technique.

It would be much more useful to indicate where the values are used.
Such a field/parameter could (probably) have the type of the enum.
But, at some point, the compiler might start barfing at that at well.

There are also a whole load of crappy __packed in that header file.
There might be one or two 64bit items on 32bit boundaries but
that can be solved without using __packed.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14 10:06     ` David Laight
@ 2018-08-14 11:08       ` Arnd Bergmann
  2018-08-14 11:10         ` Johannes Berg
  2018-08-14 13:09         ` David Laight
  0 siblings, 2 replies; 11+ messages in thread
From: Arnd Bergmann @ 2018-08-14 11:08 UTC (permalink / raw)
  To: David Laight
  Cc: Johannes Berg, Masahiro Yamada, Andrew Morton, Jakub Kicinski,
	Andy Shevchenko, Al Viro, Linux Kernel Mailing List

On Tue, Aug 14, 2018 at 12:04 PM David Laight <David.Laight@aculab.com> wrote:
>
> From: Johannes Berg
> > Sent: 14 August 2018 08:57
> ...
> > > How about fixing the root cause
> > > in drivers/net/wireless/intel/iwlwifi/fw/api/rx.h ?
> > >
> > >
> > > #define IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK   0x1e00000000ULL
> > >
> > >
> > > enum iwl_rx_he_phy looks really strange.
> >
> > Why? I don't think this is a problem, the enum is used here to get
> > constants so that we can also have documentation for them. That's a
> > common and accepted technique.
>
> It would be much more useful to indicate where the values are used.
> Such a field/parameter could (probably) have the type of the enum.
> But, at some point, the compiler might start barfing at that at well.

I think the compiler warning here only happens because one uses
a compile-time constant expression that is not a numeric literal value
into a boolean operator. That doesn't mean that there is something
wrong with the enum in particular, or that enums cause a lot of
issues elsewhere.

I would also argue that generally speaking the BUILD_BUG_ON_MSG()
should try to either produce the specific build failure it was designed
for, or not produce any output at all, rather than something
that is more confusing to developers. If we want to enforce
passing in number literals here, we should make that an explicit
check, or otherwise allow any compile-time constant values.

> There are also a whole load of crappy __packed in that header file.
> There might be one or two 64bit items on 32bit boundaries but
> that can be solved without using __packed.

Agreed, this likely causes problems on architectures without unaligned
load/store instructions that end up doing byte accesses to the descriptor
fields, which in turn can confuse the hardware, and can become very
slow when they live in dma_alloc_coherent() memory. That looks
like a completely unrelated issue though.

      Arnd

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14 11:08       ` Arnd Bergmann
@ 2018-08-14 11:10         ` Johannes Berg
  2018-08-14 13:09         ` David Laight
  1 sibling, 0 replies; 11+ messages in thread
From: Johannes Berg @ 2018-08-14 11:10 UTC (permalink / raw)
  To: Arnd Bergmann, David Laight
  Cc: Masahiro Yamada, Andrew Morton, Jakub Kicinski, Andy Shevchenko,
	Al Viro, Linux Kernel Mailing List

On Tue, 2018-08-14 at 13:08 +0200, Arnd Bergmann wrote:

> > It would be much more useful to indicate where the values are used.
> > Such a field/parameter could (probably) have the type of the enum.
> > But, at some point, the compiler might start barfing at that at well.
> 
> I think the compiler warning here only happens because one uses
> a compile-time constant expression that is not a numeric literal value
> into a boolean operator. That doesn't mean that there is something
> wrong with the enum in particular, or that enums cause a lot of
> issues elsewhere.
> 
> I would also argue that generally speaking the BUILD_BUG_ON_MSG()
> should try to either produce the specific build failure it was designed
> for, or not produce any output at all, rather than something
> that is more confusing to developers. If we want to enforce
> passing in number literals here, we should make that an explicit
> check, or otherwise allow any compile-time constant values.

I don't see why we should only be allowed to pass a number literal
(perhaps after pre-processing), rather than any constant integer value.
Clearly it needs to be a constant for all the constant-folding to work,
but nothing else is required.

> > There are also a whole load of crappy __packed in that header file.
> > There might be one or two 64bit items on 32bit boundaries but
> > that can be solved without using __packed.
> 
> Agreed, this likely causes problems on architectures without unaligned
> load/store instructions that end up doing byte accesses to the descriptor
> fields, which in turn can confuse the hardware, and can become very
> slow when they live in dma_alloc_coherent() memory. That looks
> like a completely unrelated issue though.

We know about this, but it doesn't cause issues apart from perhaps
somewhat slower access on some architectures, since all of the
structures live only in DRAM and are not usually used with coherent
memory, just sent to/from the device.

Since the devices really only ship on x86 systems, we decided to ignore
that slight performance issue (for now).

johannes

^ permalink raw reply	[flat|nested] 11+ messages in thread

* RE: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14 11:08       ` Arnd Bergmann
  2018-08-14 11:10         ` Johannes Berg
@ 2018-08-14 13:09         ` David Laight
  1 sibling, 0 replies; 11+ messages in thread
From: David Laight @ 2018-08-14 13:09 UTC (permalink / raw)
  To: 'Arnd Bergmann'
  Cc: Johannes Berg, Masahiro Yamada, Andrew Morton, Jakub Kicinski,
	Andy Shevchenko, Al Viro, Linux Kernel Mailing List

From: Arnd Bergmann
> Sent: 14 August 2018 12:08
...
> > There are also a whole load of crappy __packed in that header file.
> > There might be one or two 64bit items on 32bit boundaries but
> > that can be solved without using __packed.
> 
> Agreed, this likely causes problems on architectures without unaligned
> load/store instructions that end up doing byte accesses to the descriptor
> fields, which in turn can confuse the hardware, and can become very
> slow when they live in dma_alloc_coherent() memory. That looks
> like a completely unrelated issue though.

If you ever define a variable of one of those types (or embed one
in another structure that contains non-word sized items) you'll
get the entire structure misaligned.
I doubt that is what you had in mind.

Maybe you could use __packed __attribute__((aligned(4))).
But it is much better just to use a 64bit type with only 4 byte alignment
(I think there is a standard one in one of the header files).

I'd also add a compile-time assert on the length of the non-trivial
structures. That will detect all sorts of errors.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-13 22:09 [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning Arnd Bergmann
  2018-08-13 23:57 ` Masahiro Yamada
@ 2018-08-14 23:11 ` Andrew Morton
  2018-08-14 23:13   ` Andrew Morton
  1 sibling, 1 reply; 11+ messages in thread
From: Andrew Morton @ 2018-08-14 23:11 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Masahiro Yamada, Johannes Berg, Jakub Kicinski, Andy Shevchenko,
	Al Viro, linux-kernel

On Tue, 14 Aug 2018 00:09:34 +0200 Arnd Bergmann <arnd@arndb.de> wrote:

> Passing an enum into FIELD_GET() produces a long but harmless warning on
> newer compilers:
> 
>                  from include/linux/linkage.h:7,
>                  from include/linux/kernel.h:7,
>                  from include/linux/skbuff.h:17,
>                  from include/linux/if_ether.h:23,
>                  from include/linux/etherdevice.h:25,
>                  from drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:63:
> drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c: In function 'iwl_mvm_rx_mpdu_mq':
> include/linux/bitfield.h:56:20: error: enum constant in boolean context [-Werror=int-in-bool-context]
>    BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero"); \
>                     ^
> ...
> include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
>    __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
>    ^~~~~~~~~~~~~~~~
> drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
>     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,

Newer compilers will previously be used on older kernels, so I'll add a
cc:stable here.

> The problem here is that the caller has no idea how the macro gets
> expanding, leading to a false-positive. It can be trivially avoided
> by doing a comparison against zero.
> 
> This only recently started appearing as the iwlwifi driver was patched
> to use FIELD_GET.
> 
> Fixes: 514c30696fbc ("iwlwifi: add support for IEEE802.11ax")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  include/linux/bitfield.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/include/linux/bitfield.h b/include/linux/bitfield.h
> index 65a6981eef7b..3f1ef4450a7c 100644
> --- a/include/linux/bitfield.h
> +++ b/include/linux/bitfield.h
> @@ -53,7 +53,7 @@
>  	({								\
>  		BUILD_BUG_ON_MSG(!__builtin_constant_p(_mask),		\
>  				 _pfx "mask is not constant");		\
> -		BUILD_BUG_ON_MSG(!(_mask), _pfx "mask is zero");	\
> +		BUILD_BUG_ON_MSG((_mask) == 0, _pfx "mask is zero");	\
>  		BUILD_BUG_ON_MSG(__builtin_constant_p(_val) ?		\
>  				 ~((_mask) >> __bf_shf(_mask)) & (_val) : 0, \

I'm not understanding how a switch from !x to x==0 can fix anything. 
Help!


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning
  2018-08-14 23:11 ` Andrew Morton
@ 2018-08-14 23:13   ` Andrew Morton
  0 siblings, 0 replies; 11+ messages in thread
From: Andrew Morton @ 2018-08-14 23:13 UTC (permalink / raw)
  To: Arnd Bergmann, Masahiro Yamada, Johannes Berg, Jakub Kicinski,
	Andy Shevchenko, Al Viro, linux-kernel

On Tue, 14 Aug 2018 16:11:06 -0700 Andrew Morton <akpm@linux-foundation.org> wrote:

> > include/linux/bitfield.h:103:3: note: in expansion of macro '__BF_FIELD_CHECK'
> >    __BF_FIELD_CHECK(_mask, _reg, 0U, "FIELD_GET: "); \
> >    ^~~~~~~~~~~~~~~~
> > drivers/net/wireless/intel/iwlwifi/mvm/rxmq.c:1025:21: note: in expansion of macro 'FIELD_GET'
> >     le16_encode_bits(FIELD_GET(IWL_RX_HE_PHY_SIBG_SYM_OR_USER_NUM_MASK,
> 
> Newer compilers will previously be used on older kernels, so I'll add a
> cc:stable here.

No I won't.  Older kernels don't have the iwlwifi patch.

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2018-08-14 23:13 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-08-13 22:09 [PATCH] bitfield: avoid gcc-8 -Wint-in-bool-context warning Arnd Bergmann
2018-08-13 23:57 ` Masahiro Yamada
2018-08-14  7:56   ` Johannes Berg
2018-08-14  8:43     ` Arnd Bergmann
2018-08-14  9:31     ` Masahiro Yamada
2018-08-14 10:06     ` David Laight
2018-08-14 11:08       ` Arnd Bergmann
2018-08-14 11:10         ` Johannes Berg
2018-08-14 13:09         ` David Laight
2018-08-14 23:11 ` Andrew Morton
2018-08-14 23:13   ` Andrew Morton

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).