On 11.03.25 00:33, Marek Vasut wrote:
> 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 ?
We already changed all L"" strings to u"".
But the compiler complains about printf("%ls\n", (u16 *)buf); without
-fshort-wchar.
So we cannot do without the flag.
In fs/exfat/Makefile you should be able to disable the flag with
CFLAGS_REMOVE_* and AFLAGS_REMOVE_*
Best regards
Heinrich