On 10/17/25 19:08, Sean Anderson wrote:
On 10/15/25 12:28, Tom Rini wrote:
On Thu, Oct 02, 2025 at 10:36:07AM -0400, 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 <[email protected]>
---
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).
OK, but this is for 64-bit systems too, isn't it?
Sorry, I worded that poorly. What I mean is that the spec says
| On exit, R0 contains:
| the current length of the file object, if the call is successful
| -1 if an error occurs.
which implies that R0 is a signed integer (otherwise they would have
The only thing that is certain here is that the register will be filled
with (unsigned long)-1L if an error occurs and that any other value is
not an error.
Everything else is your personal interpretation which does not match the
existing implementation by Linaro in QEMU.
specified 0xffffffff or 0xffffffffffffffff as the error value). So
negative values (other than -1) indicate a negative file size (or are
unspecified).
However, both OpenOCD and QEMU just take the return from stat(2) and
copy it into R0. With a 32-bit host there is no problem since stat(2)
will itself fail for files over 2 GiB. With a 32-bit target, this
produces erroneous values for files larger than 4 GiB. Files sizes
between 2 and 4 GiB could be recovered, but I don't think we should do
that. On a 64-bit target, there is again no ambiguity (at least until 8
You still don't provide any reason why you don't want to support files
in the range [2 GiB, 4GiB[ on a 32 bit system.
Instead of restricting U-Boot we should add a fix in QEMU to report an
error if the filesize exceeds ULONG_MAX - 1.
This is the only way we can ensure that a 5 GiB file is not reported as
1 GiB in U-Boot as we see it today.
Best regards
Heinrich
EiB file sizes).
I don't think we should try to interpret negative values because it's
ambiguous in the spec, the hosts known to return such values do so
because of flaws in their implementation, and large files themselves
have dubious merit.
--Sean
