Thread (1 message) 1 message, 1 author, 2020-08-27

Re: [PATCH 08/30] net: wireless: ath: carl9170: Mark 'ar9170_qmap' as __maybe_unused

From: Kalle Valo <hidden>
Date: 2020-08-27 07:55:22
Also in: linux-wireless, lkml

Possibly related (same subject, not in this thread)

Lee Jones [off-list ref] writes:
On Mon, 17 Aug 2020, Kalle Valo wrote:
quoted
Rasmus Villemoes [off-list ref] writes:
quoted
On 14/08/2020 17.14, Christian Lamparter wrote:
quoted
On 2020-08-14 13:39, Lee Jones wrote:
quoted
'ar9170_qmap' is used in some source files which include carl9170.h,
but not all of them.  Mark it as __maybe_unused to show that this is
not only okay, it's expected.

Fixes the following W=1 kernel build warning(s)
Is this W=1 really a "must" requirement? I find it strange having
__maybe_unused in header files as this "suggests" that the
definition is redundant.
In this case it seems one could replace the table lookup with a

static inline u8 ar9170_qmap(u8 idx) { return 3 - idx; }

gcc doesn't warn about unused static inline functions (or one would have
a million warnings to deal with). Just my $0.02.
Yeah, this is much better.

And I think that static variables should not even be in the header
files. Doesn't it mean that there's a local copy of the variable
everytime the .h file is included? Sure, in this case the overhead is
small (4 bytes per include) but still it's wrong.
It happens a lot.

As I stated before, the 2 viable options are to a) move it into the
source files; ensuring code duplication, unnecessary maintenance
burden and probably disparity over time, or b) create (or locate if
there is one already) a special header file which is only to be
included by the users.

The later option gets really complicated if there are a variety of
tables which are included by any given number of source file
permutations.

The accepted answer in all of the other subsystems I've worked with so
far, is to use __maybe_unused.  It's simple, non-intrusive and doesn't
rely on any functional changes.
quoted
Having a static inline
function would solve that problem as well the compiler warning.
This time yes, but it's a hack that will only work with simple,
linear data.  
To me __maybe_unused is a hack and a static inline function is a much
better solution.
Try doing that with some of the other, more complicated
tables, like mwifiex_sdio_sd8*.
Then the table should be moved to a .c file and the .h file should have
"extern const int foo[]"

-- 
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help