[PATCH 08/12] fs: exfat: Fix conversion overflow errors

[Marek Vasut]

On 3/10/25 10:49 PM, Tom Rini wrote:
> On Mon, Mar 10, 2025 at 10:03:37PM +0100, Marek Vasut wrote:
>> On 3/10/25 12:17 PM, Quentin Schulz wrote:
>>> Hi Marek,
>>
>> Hi,
>>
>>> On 3/8/25 9:12 PM, Marek Vasut wrote:
>>>> Fix the following conversion overflow errors. The input field is already
>>>> limited to 3/2/1 bits using the bitwise and, move the parenthesis around
>>>> to avoid the bogus warning:
>>>>
>>>> "
>>>> fs/exfat/utf.c: In function ‘utf8_to_wchar’:
>>>> fs/exfat/utf.c:165:23: warning: overflow in conversion from ‘int’ to
>>>> ‘wchar_t’ {aka ‘short unsigned int’} changes value from ‘(int)(short
>>>> unsigned int)*input << 18 & 1835008’ to ‘0’ [-Woverflow]
>>>>     165 |                 *wc = ((wchar_t) input[0] & 0x07) << 18;
>>>>         |                       ^
>>>> fs/exfat/utf.c:170:23: warning: overflow in conversion from ‘int’ to
>>>> ‘wchar_t’ {aka ‘short unsigned int’} changes value from ‘(int)(short
>>>> unsigned int)*input << 24 & 50331648’ to ‘0’ [-Woverflow]
>>>>     170 |                 *wc = ((wchar_t) input[0] & 0x03) << 24;
>>>>         |                       ^
>>>> fs/exfat/utf.c:175:23: warning: overflow in conversion from ‘int’ to
>>>> ‘wchar_t’ {aka ‘short unsigned int’} changes value from ‘(int)(short
>>>> unsigned int)*input << 30 & 1073741824’ to ‘0’ [-Woverflow]
>>>>     175 |                 *wc = ((wchar_t) input[0] & 0x01) << 30;
>>>>         |                       ^
>>>> "
>>>>
>>>
>>> Since this doesn't seem to be U-Boot-specific, any chance to open a Pull
>>> Request on the project so we may be able to not carry this patch when
>>> upgrading (yes, the last commit in the branch was two years ago, but it
>>> seems the maintainer is still active on issues).
>>>
>>> Considering that wchar_t is an unsigned short int and that USHRT_MAX is
>>> 0xffff (so 2B or 16b)...
>> In fact, this error does not even appear in upstream , because upstream does
>> not use -fshort-wchar compiler flag . U-Boot does use this compiler flags
>> since 4a85663ec7ed ("kbuild: Enable -fshort-wchar") .
>>
>> So I wonder if this might be the right fix here instead:
>>
>> diff --git a/fs/exfat/Makefile b/fs/exfat/Makefile
>> index 550c0683d65..8ca112cf0e9 100644
>> --- a/fs/exfat/Makefile
>> +++ b/fs/exfat/Makefile
>> @@ -1,6 +1,9 @@
>>   # SPDX-License-Identifier: GPL-2.0+
>>   #
>>
>> +# The utf.o does really need 32bit wchar_t
>> +CFLAGS_REMOVE_utf.o := -fshort-wchar
>> +
>>   obj-$(CONFIG_FS_EXFAT) += \
>>   	cluster.o \
>>   	io.o \
> 
> Your initial approach is right I think, I'm not sure it's a good idea
> to mix and match that flag throughout the codebase.

The -fshort-wchar is needed only because of EFI , so I wonder if we 
should somehow isolate the EFI code which needs this stuff instead, and 
drop the flag throughout the codebase instead ? Heinrich ?

Or maybe EFI should switch to something else that is not wchar_t , so 
other users can use wchar_t unrestricted ?