Christian Lamparter <chunkeey@xxxxxxxxx> writes: > On Monday, June 10, 2019 9:06:30 AM CEST Kalle Valo wrote: >> Christian Lamparter <chunkeey@xxxxxxxxx> writes: >> >> > This patch fixes a noisy warning triggered by -Wenum-compare >> > >> > |main.c:1390:31: warning: comparison between ‘enum nl80211_ac’ >> > | and ‘enum ar9170_txq’ [-Wenum-compare] >> > | BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ); >> > | ^ >> > | [...] >> > >> > This is a little bit unfortunate, since the number of queues >> > (hence NL80211_NUM_ACS) is a constant based on the IEEE 802.11 >> > (much like IEEE80211_NUM_ACS) and __AR9170_NUM_TXQ is more or >> > less defined by the AR9170 hardware. >> >> Is the warning enabled by default? TBH I'm not seeing how useful this >> warning is for kernel development. > > It is included in the "-Wall" (which is coming from "KBUILD_CFLAGS" > in the main Makefile). > > I tried debian's gcc starting from 4.6 to the lastest 8.3. They all > complain about it in various degrees. > > https://gcc.gnu.org/onlinedocs/gcc-4.6.0/gcc/Warning-Options.html#Warning-Options Ok, odd that I haven't noticed this warning. Maybe I have been just blind. >> > --- a/drivers/net/wireless/ath/carl9170/main.c >> > +++ b/drivers/net/wireless/ath/carl9170/main.c >> > @@ -1387,7 +1387,7 @@ static int carl9170_op_conf_tx(struct ieee80211_hw *hw, >> > int ret; >> > >> > BUILD_BUG_ON(ARRAY_SIZE(ar9170_qmap) != __AR9170_NUM_TXQ); >> > - BUILD_BUG_ON(NL80211_NUM_ACS > __AR9170_NUM_TXQ); >> > + BUILD_BUG_ON((size_t)NL80211_NUM_ACS > (size_t)__AR9170_NUM_TXQ); >> >> IMHO this just makes the code worse. Does it make sense to workaround >> (stupid) compiler warnings like this? > > True. What's worse: This isn't really code. The BUILD_BUG_ON Macro is there > to guard but it's getting compiled away. I could also just drop it. Either way is fine for me. If the GCC (by default) emits a warning for this we need to silence that warning, so in that respect I guess we should apply this. Unless better solutions come up, of course :) -- Kalle Valo
