diff mbox series

[RFC,08/11] target/arm: correct return values for WRITE/READ in arm-semi

Message ID 20190514155301.16123-9-alex.bennee@linaro.org
State Superseded
Headers show
Series semihosting cleanup and re-factor | expand

Commit Message

Alex Bennée May 14, 2019, 3:52 p.m. UTC
The documentation says the write should return the number of bytes not
written on an error (0 means everything was written). Read provides a
buffer length and the return value should be the buffer length - bytes
actually read. Remove the incorrect FIXME's and return the correct
values.

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

---
 target/arm/arm-semi.c | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

-- 
2.20.1

Comments

Philippe Mathieu-Daudé May 14, 2019, 4:57 p.m. UTC | #1
On 5/14/19 5:52 PM, Alex Bennée wrote:
> The documentation says the write should return the number of bytes not

> written on an error (0 means everything was written). Read provides a

> buffer length and the return value should be the buffer length - bytes

> actually read. Remove the incorrect FIXME's and return the correct

> values.

> 

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


Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>


> ---

>  target/arm/arm-semi.c | 20 ++++++++++++--------

>  1 file changed, 12 insertions(+), 8 deletions(-)

> 

> diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c

> index a3bbef18ef7..3ae8f05d51b 100644

> --- a/target/arm/arm-semi.c

> +++ b/target/arm/arm-semi.c

> @@ -334,13 +334,15 @@ target_ulong do_arm_semihosting(CPUARMState *env)

>          } else {

>              s = lock_user(VERIFY_READ, arg1, len, 1);

>              if (!s) {

> -                /* FIXME - should this error code be -TARGET_EFAULT ? */

> -                return (uint32_t)-1;

> +                /* Return bytes not written on error */

> +                return len;

>              }

>              ret = set_swi_errno(ts, write(arg0, s, len));

>              unlock_user(s, arg1, 0);

> -            if (ret == (uint32_t)-1)

> -                return -1;

> +            if (ret == (uint32_t)-1) {

> +                ret = 0;

> +            }

> +            /* Return bytes not written */

>              return len - ret;

>          }

>      case TARGET_SYS_READ:

> @@ -355,15 +357,17 @@ target_ulong do_arm_semihosting(CPUARMState *env)

>          } else {

>              s = lock_user(VERIFY_WRITE, arg1, len, 0);

>              if (!s) {

> -                /* FIXME - should this error code be -TARGET_EFAULT ? */

> -                return (uint32_t)-1;

> +                /* return bytes not read */

> +                return len;

>              }

>              do {

>                  ret = set_swi_errno(ts, read(arg0, s, len));

>              } while (ret == -1 && errno == EINTR);

>              unlock_user(s, arg1, len);

> -            if (ret == (uint32_t)-1)

> -                return -1;

> +            if (ret == (uint32_t)-1) {

> +                ret = 0;

> +            }

> +            /* Return bytes not read */

>              return len - ret;

>          }

>      case TARGET_SYS_READC:

>
diff mbox series

Patch

diff --git a/target/arm/arm-semi.c b/target/arm/arm-semi.c
index a3bbef18ef7..3ae8f05d51b 100644
--- a/target/arm/arm-semi.c
+++ b/target/arm/arm-semi.c
@@ -334,13 +334,15 @@  target_ulong do_arm_semihosting(CPUARMState *env)
         } else {
             s = lock_user(VERIFY_READ, arg1, len, 1);
             if (!s) {
-                /* FIXME - should this error code be -TARGET_EFAULT ? */
-                return (uint32_t)-1;
+                /* Return bytes not written on error */
+                return len;
             }
             ret = set_swi_errno(ts, write(arg0, s, len));
             unlock_user(s, arg1, 0);
-            if (ret == (uint32_t)-1)
-                return -1;
+            if (ret == (uint32_t)-1) {
+                ret = 0;
+            }
+            /* Return bytes not written */
             return len - ret;
         }
     case TARGET_SYS_READ:
@@ -355,15 +357,17 @@  target_ulong do_arm_semihosting(CPUARMState *env)
         } else {
             s = lock_user(VERIFY_WRITE, arg1, len, 0);
             if (!s) {
-                /* FIXME - should this error code be -TARGET_EFAULT ? */
-                return (uint32_t)-1;
+                /* return bytes not read */
+                return len;
             }
             do {
                 ret = set_swi_errno(ts, read(arg0, s, len));
             } while (ret == -1 && errno == EINTR);
             unlock_user(s, arg1, len);
-            if (ret == (uint32_t)-1)
-                return -1;
+            if (ret == (uint32_t)-1) {
+                ret = 0;
+            }
+            /* Return bytes not read */
             return len - ret;
         }
     case TARGET_SYS_READC: