On 30/09/2024 17:15, Nathan Chancellor wrote:
> On Mon, Sep 30, 2024 at 03:58:06PM +0200, Javier Carrasco wrote:
>> On 30/09/2024 15:49, David Binderman wrote:
>>> Hello there,
>>>
>>> I just tried to build linux-6.12-rc1 with clang. It said:
>>>
>>> drivers/iio/imu/bmi323/bmi323_core.c:133:27: warning: variable 'bmi323_ext_reg_savestate' is not needed and will not be emitted [-Wunneeded-internal-declaration]
>>>
>>> A grep for the identifier shows the following strange results::
>>>
>>> inux-6.12-rc1 $ grep bmi323_ext_reg_savestate drivers/iio/imu/bmi323/bmi323_core.c
>>> static const unsigned int bmi323_ext_reg_savestate[] = {
>>> unsigned int ext_reg_settings[ARRAY_SIZE(bmi323_ext_reg_savestate)];
>>> for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>>> for (unsigned int i = 0; i < ARRAY_SIZE(bmi323_ext_reg_savestate); i++) {
>>> linux-6.12-rc1 $
>>>
>>> I see no mention of bmi323_ext_reg_savestate[ i]. Is there a possible
>>> cut'n'paste error in one of the two for loops ?
>>>
>>> Regards
>>>
>>> David Binderman
>>
>>
>> I think that is a bug in clang:
>>
>> https://bugs.llvm.org/show_bug.cgi?id=33068
>>
>> That happens because clang sees that bmi323_ext_reg_savestate is not
>> used but to gets its size, and that means for it that the variable is
>> not needed. That does not happen for example with
>> bmi323_ext_reg_savestate (right above bmi323_ext_reg_savestate) because
>> that one is used beyond ARRAY_SIZE.
>>
>> Safe to ignore?
>
> As later comments in this thread point out, this is a bug in the code:
>
> https://git.kernel.org/jic23/iio/c/506a1ac4c4464a61e4336e135841067dbc040aaa
>
> That bug report is pretty misguided, as that is exactly the type of code
> that warning tries to catch: the programmer has a static variable that
> is only used in sizeof(), a compile time evaluation, so they won't get a
> -Wunused-variable since it is used in that expression, but did they
> intend to use it elsewhere? If they didn't, they can just slap
> '__maybe_unused' / '__attribute__((unused))' on it or use
> sizeof(<type>), which removes all ambiguity. Otherwise, the code should
> be fixed, like in this case.
>
> Cheers,
> Nathan
@Nathan thanks for the clarification.
So in this case, the array should have been used not only to get its size.
But if that wasn't the case, and since you can't use sizeof(<type>),
should it be marked with __maybe_unused / __attribute__((unused)) even
though it's known in advance that it won't be used, or at least that its
use will be to get its size? Is it then just to silence the warning, or
does it have other implications? Thanks again!
