Re: [RFC PATCH 1/2] audio/mixeng: Fix Clang 'int-conversion' warning

[Richard Henderson]

On 5/3/20 4:32 AM, Philippe Mathieu-Daudé wrote:
> When building with Clang 10 on Fedora 32, we get:
> 
>     CC      audio/mixeng.o
>   audio/mixeng.c:274:34: error: implicit conversion from 'unsigned int' to 
> 'float' changes value from 4294967295 to 4294967296 
> [-Werror,-Wimplicit-int-float-conversion]
>   static const float float_scale = UINT_MAX / 2.f;
>                                    ^~~~~~~~ ~
>   /usr/lib64/clang/10.0.0/include/limits.h:56:37: note: expanded from macro 
> 'UINT_MAX'
>   #define UINT_MAX  (__INT_MAX__  *2U +1U)
>                      ~~~~~~~~~~~~~~~~~^~~
> 
> Fix by using a 64-bit float for the conversion, before casting
> back to 32-bit float.
> 
> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
> ---
>  audio/mixeng.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

No, this should be fixed properly.

First, the warning is in the !FLOAT_MIXENG branch.  IMO that means we should
not be using floating point at all, and this should be a simple integral
multiply/shift.

I had a brief look at this before the 5.0 release.  The arithmetic all through
audio looks confused to me.  There's a combination of shifting and masking
(implying a scale by 1<<32), and multiplication and division by UINT32_MAX.

I'm reasonably certain that every appearance of UINT32_MAX in this code is an
off-by-one bug, or a misuse of the constant.


r~