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

[Heinrich Schuchardt]

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.

Best let's test the real behavior on QEMU with such a large file.

Best regards

Heinrich