Re: [PATCH 2/3] semihosting: Fix GDB File-I/O FLEN

["Alex Bennée" <alex.bennee@linaro.org>]

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