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

[Andrew Goodbody]

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.
> 
> %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,
>