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

[Andrew Goodbody]

On 02/10/2025 12:03, Heinrich Schuchardt wrote:
> On 10/2/25 13:00, Andrew Goodbody wrote:
>> On 02/10/2025 10: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://developer.arm.com/documentation/ 
>>> dui0203/ j/semihosting/semihosting-operations/sys-flen--0x0c-) 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.
> 
> How about changing the smh_flen() interface to:
> 
> int smh_flen(int fd, size_t *size)

V2 coming.

Andrew

> Best regards
> 
> Heinrich
> 
>>>
>>> %s/if (size < 0)/if (size == -1L)/
>>
>> Unfortunately that is not sufficient with the current code. smh_flen() 
>> and others will call smh_errno() if the sys call returns -1 so other 
>> error codes may possibly be returned. [1]
>> This seems to be going well beyond the scope of my patch.
>>
>> Andrew
>>
>> 1. 80e62ccfa63 arm: smh: Return errno on error
>>
>>> Best regards
>>>
>>> Heinrich
>>>
>>>>               smh_close(fd);
>>>>               return size;
>>>>           }
>>>>
>>>> ---
>>>> base-commit: da47ddebd16a7e1047da8537fbf01558d2a89fcf
>>>> change-id: 20251002-fs_semihosting-85d697fbfcad
>>>>
>>>> Best regards,
>>>
>>
>