[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [RFC PATCH 1/2] audio/mixeng: Fix Clang 'int-conversion' warning
From: |
Richard Henderson |
Subject: |
Re: [RFC PATCH 1/2] audio/mixeng: Fix Clang 'int-conversion' warning |
Date: |
Sun, 3 May 2020 10:12:58 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0 |
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~