Re: [PATCH fifo 2/2] Cygwin: FIFO: add support for the duplex case

2019-03-30 Thread Ken Brown
Hi Takashi,

On 3/30/2019 9:30 AM, Takashi Yano wrote:
> Hi Ken,
> 
> Do these patches enable to open FIFO multiple times with O_RDWR?
> mc (midnight commander) tries to open FIFO twice with O_RDWR if
> SHELL=/bin/tcsh, but fails.
> https://cygwin.com/ml/cygwin/2017-03/msg00188.html

No, I'm sorry.  At the moment, a FIFO can only be opened once for reading.  I 
plan to work on this next, but first I have some bugs to fix.

Ken


Re: [PATCH fifo 2/2] Cygwin: FIFO: add support for the duplex case

2019-03-30 Thread Takashi Yano
Hi Ken,

Do these patches enable to open FIFO multiple times with O_RDWR?
mc (midnight commander) tries to open FIFO twice with O_RDWR if
SHELL=/bin/tcsh, but fails.
https://cygwin.com/ml/cygwin/2017-03/msg00188.html

On Mon, 25 Mar 2019 23:06:10 + Ken Brown wrote:
> If a FIFO is opened with O_RDWR access, create the pipe with
> read/write access, and make the first client have the handle of that
> pipe as its I/O handle.
> 
> Adjust fhandler_fifo::raw_read to account for the result of trying to
> read from that client if there's no data.
> ---
>  winsup/cygwin/fhandler.h   |  5 +++
>  winsup/cygwin/fhandler_fifo.cc | 79 +-
>  2 files changed, 73 insertions(+), 11 deletions(-)
> 
> diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
> index ef34f9c40..3398cc625 100644
> --- a/winsup/cygwin/fhandler.h
> +++ b/winsup/cygwin/fhandler.h
> @@ -1253,6 +1253,10 @@ struct fifo_client_handler
>HANDLE dummy_evt;  /* Never signaled. */
>fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
>  dummy_evt (NULL) {}
> +  fifo_client_handler (fhandler_base *_fh, fifo_client_connect_state _state,
> +HANDLE _connect_evt, HANDLE _dummy_evt)
> +: fh (_fh), state (_state), connect_evt (_connect_evt),
> +  dummy_evt (_dummy_evt) {}
>int connect ();
>int close ();
>  };
> @@ -1268,6 +1272,7 @@ class fhandler_fifo: public fhandler_base
>fifo_client_handler client[MAX_CLIENTS];
>int nclients, nconnected;
>af_unix_spinlock_t _fifo_client_lock;
> +  bool _duplexer;
>bool __reg2 wait (HANDLE);
>NTSTATUS npfs_handle (HANDLE &);
>HANDLE create_pipe_instance (bool);
> diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
> index 2c20444c6..7847cca82 100644
> --- a/winsup/cygwin/fhandler_fifo.cc
> +++ b/winsup/cygwin/fhandler_fifo.cc
> @@ -33,7 +33,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. 
> */
>  fhandler_fifo::fhandler_fifo ():
>fhandler_base (), read_ready (NULL), write_ready (NULL),
>listen_client_thr (NULL), lct_termination_evt (NULL), nclients (0),
> -  nconnected (0)
> +  nconnected (0), _duplexer (false)
>  {
>pipe_name_buf[0] = L'\0';
>need_fork_fixup (true);
> @@ -224,6 +224,8 @@ fhandler_fifo::create_pipe_instance (bool first)
>  }
>access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
>  | SYNCHRONIZE;
> +  if (first && _duplexer)
> +access |= GENERIC_WRITE;
>sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
>hattr = OBJ_INHERIT;
>if (first)
> @@ -437,7 +439,7 @@ fhandler_fifo::open (int flags, mode_t)
>  case O_RDWR:
>reader = true;
>writer = false;
> -  duplexer = true;
> +  duplexer = _duplexer = true;
>break;
>  default:
>set_errno (EINVAL);
> @@ -447,7 +449,7 @@ fhandler_fifo::open (int flags, mode_t)
>  
>debug_only_printf ("reader %d, writer %d, duplexer %d", reader, writer, 
> duplexer);
>set_flags (flags);
> -  if (reader)
> +  if (reader && !duplexer)
>  nohandle (true);
>  
>/* Create control events for this named pipe */
> @@ -472,6 +474,48 @@ fhandler_fifo::open (int flags, mode_t)
>goto out;
>  }
>  
> +  /* If we're a duplexer, create the pipe and the first client. */
> +  if (duplexer)
> +{
> +  HANDLE ph, connect_evt, dummy_evt;
> +  fhandler_base *fh;
> +
> +  ph = create_pipe_instance (true);
> +  if (!ph)
> + {
> +   res = error_errno_set;
> +   goto out;
> + }
> +  set_io_handle (ph);
> +  set_pipe_non_blocking (ph, true);
> +  if (!(fh = build_fh_dev (dev (
> + {
> +   set_errno (EMFILE);
> +   res = error_errno_set;
> +   goto out;
> + }
> +  fh->set_io_handle (ph);
> +  fh->set_flags (flags);
> +  if (!(connect_evt = create_event ()))
> + {
> +   res = error_errno_set;
> +   fh->close ();
> +   delete fh;
> +   goto out;
> + }
> +  if (!(dummy_evt = create_event ()))
> + {
> +   res = error_errno_set;
> +   delete fh;
> +   fh->close ();
> +   CloseHandle (connect_evt);
> +   goto out;
> + }
> +  client[0] = fifo_client_handler (fh, fc_connected, connect_evt,
> +dummy_evt);
> +  nconnected = nclients = 1;
> +}
> +
>/* If we're reading, start the listen_client thread (which should
>   signal read_ready), and wait for a writer. */
>if (reader)
> @@ -482,8 +526,8 @@ fhandler_fifo::open (int flags, mode_t)
> res = error_errno_set;
> goto out;
>   }
> -  /* Wait for the listen_client thread to create the pipe and
> -  signal read_ready.  This should be quick.  */
> +  /* Wait for the listen_client thread to signal read_ready.  This
> +  should be quick.  */
>HANDLE w[2] = { listen_client_thr, read_ready };
>switch 

[PATCH fifo 2/2] Cygwin: FIFO: add support for the duplex case

2019-03-25 Thread Ken Brown
If a FIFO is opened with O_RDWR access, create the pipe with
read/write access, and make the first client have the handle of that
pipe as its I/O handle.

Adjust fhandler_fifo::raw_read to account for the result of trying to
read from that client if there's no data.
---
 winsup/cygwin/fhandler.h   |  5 +++
 winsup/cygwin/fhandler_fifo.cc | 79 +-
 2 files changed, 73 insertions(+), 11 deletions(-)

diff --git a/winsup/cygwin/fhandler.h b/winsup/cygwin/fhandler.h
index ef34f9c40..3398cc625 100644
--- a/winsup/cygwin/fhandler.h
+++ b/winsup/cygwin/fhandler.h
@@ -1253,6 +1253,10 @@ struct fifo_client_handler
   HANDLE dummy_evt;/* Never signaled. */
   fifo_client_handler () : fh (NULL), state (fc_unknown), connect_evt (NULL),
   dummy_evt (NULL) {}
+  fifo_client_handler (fhandler_base *_fh, fifo_client_connect_state _state,
+  HANDLE _connect_evt, HANDLE _dummy_evt)
+: fh (_fh), state (_state), connect_evt (_connect_evt),
+  dummy_evt (_dummy_evt) {}
   int connect ();
   int close ();
 };
@@ -1268,6 +1272,7 @@ class fhandler_fifo: public fhandler_base
   fifo_client_handler client[MAX_CLIENTS];
   int nclients, nconnected;
   af_unix_spinlock_t _fifo_client_lock;
+  bool _duplexer;
   bool __reg2 wait (HANDLE);
   NTSTATUS npfs_handle (HANDLE &);
   HANDLE create_pipe_instance (bool);
diff --git a/winsup/cygwin/fhandler_fifo.cc b/winsup/cygwin/fhandler_fifo.cc
index 2c20444c6..7847cca82 100644
--- a/winsup/cygwin/fhandler_fifo.cc
+++ b/winsup/cygwin/fhandler_fifo.cc
@@ -33,7 +33,7 @@ STATUS_PIPE_EMPTY simply means there's no data to be read. */
 fhandler_fifo::fhandler_fifo ():
   fhandler_base (), read_ready (NULL), write_ready (NULL),
   listen_client_thr (NULL), lct_termination_evt (NULL), nclients (0),
-  nconnected (0)
+  nconnected (0), _duplexer (false)
 {
   pipe_name_buf[0] = L'\0';
   need_fork_fixup (true);
@@ -224,6 +224,8 @@ fhandler_fifo::create_pipe_instance (bool first)
 }
   access = GENERIC_READ | FILE_READ_ATTRIBUTES | FILE_WRITE_ATTRIBUTES
 | SYNCHRONIZE;
+  if (first && _duplexer)
+access |= GENERIC_WRITE;
   sharing = FILE_SHARE_READ | FILE_SHARE_WRITE;
   hattr = OBJ_INHERIT;
   if (first)
@@ -437,7 +439,7 @@ fhandler_fifo::open (int flags, mode_t)
 case O_RDWR:
   reader = true;
   writer = false;
-  duplexer = true;
+  duplexer = _duplexer = true;
   break;
 default:
   set_errno (EINVAL);
@@ -447,7 +449,7 @@ fhandler_fifo::open (int flags, mode_t)
 
   debug_only_printf ("reader %d, writer %d, duplexer %d", reader, writer, 
duplexer);
   set_flags (flags);
-  if (reader)
+  if (reader && !duplexer)
 nohandle (true);
 
   /* Create control events for this named pipe */
@@ -472,6 +474,48 @@ fhandler_fifo::open (int flags, mode_t)
   goto out;
 }
 
+  /* If we're a duplexer, create the pipe and the first client. */
+  if (duplexer)
+{
+  HANDLE ph, connect_evt, dummy_evt;
+  fhandler_base *fh;
+
+  ph = create_pipe_instance (true);
+  if (!ph)
+   {
+ res = error_errno_set;
+ goto out;
+   }
+  set_io_handle (ph);
+  set_pipe_non_blocking (ph, true);
+  if (!(fh = build_fh_dev (dev (
+   {
+ set_errno (EMFILE);
+ res = error_errno_set;
+ goto out;
+   }
+  fh->set_io_handle (ph);
+  fh->set_flags (flags);
+  if (!(connect_evt = create_event ()))
+   {
+ res = error_errno_set;
+ fh->close ();
+ delete fh;
+ goto out;
+   }
+  if (!(dummy_evt = create_event ()))
+   {
+ res = error_errno_set;
+ delete fh;
+ fh->close ();
+ CloseHandle (connect_evt);
+ goto out;
+   }
+  client[0] = fifo_client_handler (fh, fc_connected, connect_evt,
+  dummy_evt);
+  nconnected = nclients = 1;
+}
+
   /* If we're reading, start the listen_client thread (which should
  signal read_ready), and wait for a writer. */
   if (reader)
@@ -482,8 +526,8 @@ fhandler_fifo::open (int flags, mode_t)
  res = error_errno_set;
  goto out;
}
-  /* Wait for the listen_client thread to create the pipe and
-signal read_ready.  This should be quick.  */
+  /* Wait for the listen_client thread to signal read_ready.  This
+should be quick.  */
   HANDLE w[2] = { listen_client_thr, read_ready };
   switch (WaitForMultipleObjects (2, w, FALSE, INFINITE))
{
@@ -703,12 +747,25 @@ fhandler_fifo::raw_read (void *in_ptr, size_t& len)
fifo_client_unlock ();
return;
  }
-   else if (nread < 0 && GetLastError () != ERROR_NO_DATA)
- {
-   fifo_client_unlock ();
-   goto errout;
- }
-   else if (nread == 0) /* Client has disconnected. */
+   /* In the duplex case