public inbox for cygwin-cvs@sourceware.org
help / color / mirror / Atom feed
From: Corinna Vinschen <corinna@sourceware.org>
To: cygwin-cvs@sourceware.org
Subject: [newlib-cygwin] Cygwin: pipes: fix POSIX requirement for non-blocking pipe writes
Date: Tue, 14 Sep 2021 15:06:14 +0000 (GMT)	[thread overview]
Message-ID: <20210914150614.70DA13858031@sourceware.org> (raw)

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=a5b2c735e6f4968158d286740e127187abde5e50

commit a5b2c735e6f4968158d286740e127187abde5e50
Author: Corinna Vinschen <corinna@vinschen.de>
Date:   Fri Sep 3 10:45:49 2021 +0200

    Cygwin: pipes: fix POSIX requirement for non-blocking pipe writes
    
    POSIX requires atomicity for non-blocking writes <= PIPE_BUF bytes
    and writing of at least 1 byte if any buffer space is left.
    Windows NtWriteFile returns STATUS_SUCCESS and "0 bytes written"
    if the write doesn't match buffer space.  Fix this discrepancy.
    
    Signed-off-by: Corinna Vinschen <corinna@vinschen.de>

Diff:
---
 winsup/cygwin/fhandler_pipe.cc | 51 ++++++++++++++++++++++++++++++------------
 1 file changed, 37 insertions(+), 14 deletions(-)

diff --git a/winsup/cygwin/fhandler_pipe.cc b/winsup/cygwin/fhandler_pipe.cc
index b69bb1d40..06d989772 100644
--- a/winsup/cygwin/fhandler_pipe.cc
+++ b/winsup/cygwin/fhandler_pipe.cc
@@ -313,7 +313,6 @@ fhandler_pipe::raw_read (void *ptr, size_t& len)
 ssize_t __reg3
 fhandler_pipe::raw_write (const void *ptr, size_t len)
 {
-  ssize_t ret = -1;
   size_t nbytes = 0;
   ULONG chunk;
   NTSTATUS status = STATUS_SUCCESS;
@@ -351,9 +350,36 @@ fhandler_pipe::raw_write (const void *ptr, size_t len)
 	len1 = chunk;
       else
 	len1 = (ULONG) left;
-      nbytes_now = 0;
-      status = NtWriteFile (get_handle (), evt, NULL, NULL, &io,
-			    (PVOID) ptr, len1, NULL, NULL);
+      /* NtWriteFile returns success with # of bytes written == 0 if writing
+         on a non-blocking pipe fails because the pipe buffer doesn't have
+	 sufficient space.
+
+	 POSIX requires
+	 - A write request for {PIPE_BUF} or fewer bytes shall have the
+	   following effect: if there is sufficient space available in the
+	   pipe, write() shall transfer all the data and return the number
+	   of bytes requested. Otherwise, write() shall transfer no data and
+	   return -1 with errno set to [EAGAIN].
+
+	 - A write request for more than {PIPE_BUF} bytes shall cause one
+	   of the following:
+
+	  - When at least one byte can be written, transfer what it can and
+	    return the number of bytes written. When all data previously
+	    written to the pipe is read, it shall transfer at least {PIPE_BUF}
+	    bytes.
+
+	  - When no data can be written, transfer no data, and return -1 with
+	    errno set to [EAGAIN]. */
+      while (len1 > 0)
+	{
+	  status = NtWriteFile (get_handle (), evt, NULL, NULL, &io,
+				(PVOID) ptr, len1, NULL, NULL);
+	  if (evt || !NT_SUCCESS (status) || io.Information > 0
+	      || len <= PIPE_BUF)
+	    break;
+	  len1 >>= 1;
+	}
       if (evt && status == STATUS_PENDING)
 	{
 	  waitret = cygwait (evt);
@@ -375,13 +401,11 @@ fhandler_pipe::raw_write (const void *ptr, size_t len)
       else if (NT_SUCCESS (status))
 	{
 	  nbytes_now = io.Information;
-	  /* NtWriteFile returns success with # of bytes written == 0
-	     if writing on a non-blocking pipe fails because the pipe
-	     buffer doesn't have sufficient space. */
-	  if (nbytes_now == 0)
-	    set_errno (EAGAIN);
 	  ptr = ((char *) ptr) + nbytes_now;
 	  nbytes += nbytes_now;
+	  /* 0 bytes returned?  EAGAIN.  See above. */
+	  if (nbytes == 0)
+	    set_errno (EAGAIN);
 	}
       else if (STATUS_PIPE_IS_CLOSED (status))
 	{
@@ -392,17 +416,16 @@ fhandler_pipe::raw_write (const void *ptr, size_t len)
 	__seterrno_from_nt_status (status);
 
       if (nbytes_now == 0)
-	len = 0;		/* Terminate loop. */
-      if (nbytes > 0)
-	ret = nbytes;
+	break;
     }
   if (evt)
     CloseHandle (evt);
-  if (status == STATUS_THREAD_SIGNALED && ret < 0)
+  if (status == STATUS_THREAD_SIGNALED && nbytes == 0)
     set_errno (EINTR);
   else if (status == STATUS_THREAD_CANCELED)
     pthread::static_cancel_self ();
-  return ret;
+  return nbytes ?: -1;
+}
 }
 
 int


                 reply	other threads:[~2021-09-14 15:06 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210914150614.70DA13858031@sourceware.org \
    --to=corinna@sourceware.org \
    --cc=cygwin-cvs@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).