[PATCH 13/14] Cygwin: FIFO: improve raw_write

[Corinna Vinschen]

Hi Ken,

On Apr 14 19:16, Ken Brown wrote:
> Don't set the write end of the pipe to non-blocking mode if the FIFO
> is opened in blocking mode.
> 
> In fhandler_fifo::raw_write in blocking mode, wait for the write to
> complete rather than returning -1 with EAGAIN.
> 
> If the amount to write is large, write in smaller chunks (of size
> determined by a new data member max_atomic_write), as in
> fhandler_base_overlapped.
> ---
>  winsup/cygwin/fhandler.h       |  1 +
>  winsup/cygwin/fhandler_fifo.cc | 96 +++++++++++++++++++++++++++-------
>  2 files changed, 79 insertions(+), 18 deletions(-)
> 
> [...]
> diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
> index fe4c67bdf..ce078d74d 100644
> --- a/winsup/cygwin/fhandler_fifo.cc
> +++ b/winsup/cygwin/fhandler_fifo.cc
> @@ -22,6 +22,9 @@
>  #include "ntdll.h"
>  #include "cygwait.h"
>  
> +#define STATUS_THREAD_SIGNALED	((NTSTATUS)0xE0000001)
> +#define STATUS_THREAD_CANCELED	((NTSTATUS)0xE0000002)

We want to move this to ntdll.h at one point, with a comment about
"cygwin-only" etc.  If you want to do that right now, feel free.

There's a small problem in this patch:

>  {
>    ssize_t ret = -1;
> +  size_t nbytes = 0, chunk;
>    NTSTATUS status;
     ^^^^^^^^^^^^^^^^

>    IO_STATUS_BLOCK io;
> +  HANDLE evt = NULL;
> [...]
> +  if (status == STATUS_THREAD_SIGNALED && !_my_tls.call_signal_handler ())
> +    set_errno (EINTR);
> +  else if (status == STATUS_THREAD_CANCELED)
> +    pthread::static_cancel_self ();

fhandler_fifo.cc: In member function ‘virtual ssize_t fhandler_fifo::raw_write(const void*, size_t)’:
fhandler_fifo.cc:711:8: error: ‘status’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
   else if (status == STATUS_THREAD_CANCELED)
        ^~

You only see this with -O2.  There's actually a chance this may occur if
the incoming len is 0.  Can you check, especially in terms of the
default value status should start up with here?

The other patches look good, just send a v2 of this single patch is all.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://cygwin.com/pipermail/cygwin-patches/attachments/20190415/2d32f399/attachment.sig>