fstat returns 0 on success and -1 on error. Since we have already
checked for error, ret must be zero. Therefore, any call to fstat on a
non-empty file will return -1/EOVERFLOW.

Restore the original logic that just did a byteswap. I don't really know
what the intention of the fixed commit was.

Fixes: a6300ed6b7 ("semihosting: Split out semihost_sys_flen")
Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
---

 semihosting/arm-compat-semi.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
index 6100126796..c5a07cb947 100644
--- a/semihosting/arm-compat-semi.c
+++ b/semihosting/arm-compat-semi.c
@@ -316,10 +316,7 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err)
                                 &size, 8, 0)) {
             ret = -1, err = EFAULT;
         } else {
-            size = be64_to_cpu(size);
-            if (ret != size) {
-                ret = -1, err = EOVERFLOW;
-            }
+            ret = be64_to_cpu(size);
         }
     }
     common_semi_cb(cs, ret, err);
-- 
2.35.1.1320.gc452695387.dirty

Sean Anderson <sean.anderson@linux.dev> writes:

> fstat returns 0 on success and -1 on error. Since we have already
> checked for error, ret must be zero. Therefore, any call to fstat on a
> non-empty file will return -1/EOVERFLOW.
>
> Restore the original logic that just did a byteswap. I don't really know
> what the intention of the fixed commit was.
>
> Fixes: a6300ed6b7 ("semihosting: Split out semihost_sys_flen")
> Signed-off-by: Sean Anderson <sean.anderson@linux.dev>
> ---
>
>  semihosting/arm-compat-semi.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/semihosting/arm-compat-semi.c b/semihosting/arm-compat-semi.c
> index 6100126796..c5a07cb947 100644
> --- a/semihosting/arm-compat-semi.c
> +++ b/semihosting/arm-compat-semi.c
> @@ -316,10 +316,7 @@ common_semi_flen_fstat_cb(CPUState *cs, uint64_t ret, int err)
>                                  &size, 8, 0)) {
>              ret = -1, err = EFAULT;
>          } else {
> -            size = be64_to_cpu(size);
> -            if (ret != size) {
> -                ret = -1, err = EOVERFLOW;
> -            }
> +            ret = be64_to_cpu(size);
>          }
>      }
>      common_semi_cb(cs, ret, err);

well this sent me on a rat hole to figure out the be64_to_cpu. Really I
think the callback function should be renamed to gdb_semi_flen_fstat_cb
because it is only called for the GDB File I/O implementation.

Otherwise the logic looks fine:

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro