Re: [PATCH] Fix return value on aio_read/write success

2018-08-04 Thread Mark Geisert

Corinna Vinschen wrote:

On Aug  4 01:44, Mark Geisert wrote:

Oops. Something that iozone testing had found but I regarded as an
iozone bug.  Re-reading the man pages set me straight.
---
 winsup/cygwin/aio.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/aio.cc b/winsup/cygwin/aio.cc
index fe63dec04..571a9621b 100644
--- a/winsup/cygwin/aio.cc
+++ b/winsup/cygwin/aio.cc
@@ -712,7 +712,7 @@ aio_read (struct aiocb *aio)
   ; /* I think this is not possible */
 }

-  return res;
+  return res < 0 ? res : 0; /* Return 0 on success, not byte count */


The comment only makes sense in comparison to the former code.
I'd reduce this to just "Return 0 on success".


OKWD (Okay, will do).




 }

 ssize_t
@@ -902,7 +902,7 @@ aio_write (struct aiocb *aio)
   ; /* I think this is not possible */
 }

-  return res;
+  return res < 0 ? res : 0; /* Return 0 on success, not byte count */


Ditto.


Yup.




 }

 int
--
2.17.0


While we're at it, I just found that asyncread/asyncwrite return int.
Shouldn't they return ssize_t?  That's 32 vs. 64 bit on x86_64.


Oops++.  Yes, of course.  Will do.  Saw no compiler warning for this.
Thanks,

..mark



Re: [PATCH] Fix return value on aio_read/write success

2018-08-04 Thread Corinna Vinschen
On Aug  4 01:44, Mark Geisert wrote:
> Oops. Something that iozone testing had found but I regarded as an
> iozone bug.  Re-reading the man pages set me straight.
> ---
>  winsup/cygwin/aio.cc | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/winsup/cygwin/aio.cc b/winsup/cygwin/aio.cc
> index fe63dec04..571a9621b 100644
> --- a/winsup/cygwin/aio.cc
> +++ b/winsup/cygwin/aio.cc
> @@ -712,7 +712,7 @@ aio_read (struct aiocb *aio)
>; /* I think this is not possible */
>  }
>  
> -  return res;
> +  return res < 0 ? res : 0; /* Return 0 on success, not byte count */

The comment only makes sense in comparison to the former code.
I'd reduce this to just "Return 0 on success".

>  }
>  
>  ssize_t
> @@ -902,7 +902,7 @@ aio_write (struct aiocb *aio)
>; /* I think this is not possible */
>  }
>  
> -  return res;
> +  return res < 0 ? res : 0; /* Return 0 on success, not byte count */

Ditto.

>  }
>  
>  int
> -- 
> 2.17.0

While we're at it, I just found that asyncread/asyncwrite return int.
Shouldn't they return ssize_t?  That's 32 vs. 64 bit on x86_64.


Thanks,
Corinna

-- 
Corinna Vinschen  Please, send mails regarding Cygwin to
Cygwin Maintainer cygwin AT cygwin DOT com
Red Hat


signature.asc
Description: PGP signature


[PATCH] Fix return value on aio_read/write success

2018-08-04 Thread Mark Geisert
Oops. Something that iozone testing had found but I regarded as an
iozone bug.  Re-reading the man pages set me straight.
---
 winsup/cygwin/aio.cc | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/winsup/cygwin/aio.cc b/winsup/cygwin/aio.cc
index fe63dec04..571a9621b 100644
--- a/winsup/cygwin/aio.cc
+++ b/winsup/cygwin/aio.cc
@@ -712,7 +712,7 @@ aio_read (struct aiocb *aio)
   ; /* I think this is not possible */
 }
 
-  return res;
+  return res < 0 ? res : 0; /* Return 0 on success, not byte count */
 }
 
 ssize_t
@@ -902,7 +902,7 @@ aio_write (struct aiocb *aio)
   ; /* I think this is not possible */
 }
 
-  return res;
+  return res < 0 ? res : 0; /* Return 0 on success, not byte count */
 }
 
 int
-- 
2.17.0