[PATCH] fs: semihosting: Use correct variable for error check

[Sean Anderson]

On 10/2/25 11:23, Heinrich Schuchardt wrote:
> On 10/2/25 17:15, Sean Anderson wrote:
>> On 10/2/25 11:07, Heinrich Schuchardt wrote:
>>> On 10/2/25 16:36, Sean Anderson wrote:
>>>> On 10/2/25 05:52, Heinrich Schuchardt wrote:
>>>>> On 10/2/25 11:39, Andrew Goodbody wrote:
>>>>>> After calling a function that can return an error, the test to detect
>>>>>> that error should use the return value not a different variable. Fix it.
>>>>>>
>>>>>> This issue was found by Smatch.
>>>>>>
>>>>>
>>>>> Fixes: f676b45151c3 ("fs: Add semihosting filesystem")
>>>>>
>>>>>> Signed-off-by: Andrew Goodbody <andrew.goodbody at linaro.org>
>>>>>> ---
>>>>>>     fs/semihostingfs.c | 2 +-
>>>>>>     1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/fs/semihostingfs.c b/fs/semihostingfs.c
>>>>>> index 77e39ca407e4d240a1fd573497c5b6b908816454..9d7a136b9ba9b035545b34b31df58e2d65de7db9 100644
>>>>>> --- a/fs/semihostingfs.c
>>>>>> +++ b/fs/semihostingfs.c
>>>>>> @@ -35,7 +35,7 @@ static int smh_fs_read_at(const char *filename, loff_t pos, void *buffer,
>>>>>>         }
>>>>>>         if (!maxsize) {
>>>>>>             size = smh_flen(fd);
>>>>>> -        if (ret < 0) {
>>>>>> +        if (size < 0) {
>>>>>
>>>>> The ARM specification (https://cas5-0-urlprotect.trendmicro.com:443/wis/clicktime/v1/query?url=https%3a%2f%2fdeveloper.arm.com%2fdocumentation%2fdui0203%2fj%2fsemihosting%2fsemihosting%2doperations%2fsys%2dflen%2d%2d0x0c%2d&umid=f17ab8fc-2d9d-4cce-91e2-e7e66f117613&rct=1759398729&auth=d807158c60b7d2502abde8a2fc01f40662980862-e0799d2d4d61f93cae033d54733c8fa50ef84918) has:
>>>>>
>>>>> SYS_FLEN (0x0C)
>>>>>
>>>>> Returns the length of a specified file.
>>>>> On exit, R0 contains:
>>>>>       the current length of the file object, if the call is successful
>>>>>       -1 if an error occurs.
>>>>>
>>>>> Please, consider that the file length on 32bit systems may exceed 2^31 You must not consider this as an error.
>>>>>
>>>>> %s/if (size < 0)/if (size == -1L)/
>>>>
>>>> This cannot occur because on a 32-bit system SYS_FLEN only returns
>>>> 32-bits of information. The host must detect files that exceed 2 GiB in
>>>> size and return -1 (simulating EOVERFLOW).
>>>
>>> I don't think so.
>>>
>>> QEMU will read into the st_size field of a struct stat and copy that value to the return value. The st_size field will have at least 64 bits on a 64 bit system. And copying to the 32 bit target will simply truncate it.
>>>
>>> Expect overruns for files that are larger than 0xffffffff.
>>
>> Yes. And there's nothing we can do about this. If there is a 9 GiB file,
>> SYS_FLEN will report it as a 1 GiB file. This is a bug in QEMU (and
>> OpenOCD too) that we shouldn't attempt to work around in U-Boot.
>>
>> --Sean
> 
> We should handle a 2.5 GiB file correctly and not treat is an an error.
> Such a large file could be an installation ISO that the EFI subsystem passes to Linux as memory block device (pmem).

And what about a 4.4 GiB DVD iso?

The flen interface is fundamentally limited on 32-bit systems. There is
no sense in working around it. Especially because reading such large
values is certain to take ages over JTAG, and there are far-better
interfaces to use on emulated hardware (e.g. virtio file system device).

--Sean