Hi Takashi, On Fri, 27 Jun 2025, Takashi Yano wrote:
> ... rather than 1 when the pipe space estimation fails, so that > select() and raw_wrie() can perform appropriate fallback handling. > In select(), even if pipe space is unknown, return writable to avoid > deadlock. Even with select() returns writable, write() can blocked > anyway, if data is larger than pipe space. In raw_write(), if the > pipe is real non-blocking mode, attempting to write larger data than > pipe space is safe. Otherwise, return error. I am okay with this patch as-is. The only minor thing is that neither the commit message nor the diff make it clear that all six callsites of `pipe_data_available()` are handled (the first two callsites in `peek_pipe()` are both handled by the 4th hunk of the `select.cc` diff). Thank you for fixing this! Johannes > > Reviewed-by: > Signed-off-by: Takashi Yano <[email protected]> > --- > winsup/cygwin/fhandler/pipe.cc | 7 ++++--- > winsup/cygwin/local_includes/select.h | 3 +++ > winsup/cygwin/select.cc | 18 +++++++++--------- > 3 files changed, 16 insertions(+), 12 deletions(-) > > diff --git a/winsup/cygwin/fhandler/pipe.cc b/winsup/cygwin/fhandler/pipe.cc > index 22addbb18..7e2c1861b 100644 > --- a/winsup/cygwin/fhandler/pipe.cc > +++ b/winsup/cygwin/fhandler/pipe.cc > @@ -663,12 +663,13 @@ fhandler_pipe_fifo::raw_write (const void *ptr, size_t > len) > in a very implementation-defined way we can't emulate, but it > resembles it closely enough to get useful results. */ > avail = pipe_data_available (-1, this, get_handle (), PDA_WRITE); > - if (avail < 1) /* error or pipe closed */ > + if (avail == PDA_UNKNOWN && real_non_blocking_mode) > + avail = len1; > + else if (avail == 0 || !PDA_NOERROR (avail)) > + /* error or pipe closed */ > break; > if (avail > len1) /* somebody read from the pipe */ > avail = len1; > - if (avail == 1) /* 1 byte left or non-Cygwin pipe */ > - len1 >>= 1; > else if (avail >= PIPE_BUF) > len1 = avail & ~(PIPE_BUF - 1); > else > diff --git a/winsup/cygwin/local_includes/select.h > b/winsup/cygwin/local_includes/select.h > index 43ceb1d7e..afc05e186 100644 > --- a/winsup/cygwin/local_includes/select.h > +++ b/winsup/cygwin/local_includes/select.h > @@ -143,5 +143,8 @@ ssize_t pipe_data_available (int, fhandler_base *, > HANDLE, int); > > #define PDA_READ 0x00 > #define PDA_WRITE 0x01 > +#define PDA_ERROR -1 > +#define PDA_UNKNOWN -2 > +#define PDA_NOERROR(x) (x >= 0) > > #endif /* _SELECT_H_ */ > diff --git a/winsup/cygwin/select.cc b/winsup/cygwin/select.cc > index 701f4d9a6..050221a9f 100644 > --- a/winsup/cygwin/select.cc > +++ b/winsup/cygwin/select.cc > @@ -601,7 +601,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, > int mode) > if (mode == PDA_READ > && PeekNamedPipe (h, NULL, 0, NULL, &nbytes_in_pipe, NULL)) > return nbytes_in_pipe; > - return -1; > + return PDA_ERROR; > } > > IO_STATUS_BLOCK iosb = {{0}, 0}; > @@ -618,7 +618,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, > int mode) > access on the write end. */ > select_printf ("fd %d, %s, NtQueryInformationFile failed, status %y", > fd, fh->get_name (), status); > - return (mode == PDA_WRITE) ? 1 : -1; > + return (mode == PDA_WRITE) ? PDA_UNKNOWN : PDA_ERROR; > } > if (mode == PDA_WRITE) > { > @@ -660,7 +660,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, > int mode) > return fpli.WriteQuotaAvailable; /* Not empty */ > else if (!NT_SUCCESS (status)) > /* We cannot know actual write pipe space. */ > - return 1; > + return PDA_UNKNOWN; > /* Restore pipe mode to blocking mode */ > ((fhandler_pipe *) fh)->set_pipe_non_blocking (false); > /* Empty */ > @@ -684,7 +684,7 @@ pipe_data_available (int fd, fhandler_base *fh, HANDLE h, > int mode) > return fpli.ReadDataAvailable; > } > if (fpli.NamedPipeState & FILE_PIPE_CLOSING_STATE) > - return -1; > + return PDA_ERROR; > return 0; > } > > @@ -734,7 +734,7 @@ peek_pipe (select_record *s, bool from_select) > if (n == 0 && fh->get_echo_handle ()) > n = pipe_data_available (s->fd, fh, fh->get_echo_handle (), PDA_READ); > > - if (n < 0) > + if (n == PDA_ERROR) > { > select_printf ("read: %s, n %d", fh->get_name (), n); > if (s->except_selected) > @@ -775,8 +775,8 @@ out: > } > ssize_t n = pipe_data_available (s->fd, fh, h, PDA_WRITE); > select_printf ("write: %s, n %d", fh->get_name (), n); > - gotone += s->write_ready = (n > 0); > - if (n < 0 && s->except_selected) > + gotone += s->write_ready = (n > 0 || n == PDA_UNKNOWN); > + if (n == PDA_ERROR && s->except_selected) > gotone += s->except_ready = true; > } > return gotone; > @@ -989,7 +989,7 @@ out: > ssize_t n = pipe_data_available (s->fd, fh, fh->get_handle (), > PDA_WRITE); > select_printf ("write: %s, n %d", fh->get_name (), n); > gotone += s->write_ready = (n > 0); > - if (n < 0 && s->except_selected) > + if (n == PDA_ERROR && s->except_selected) > gotone += s->except_ready = true; > } > return gotone; > @@ -1415,7 +1415,7 @@ out: > ssize_t n = pipe_data_available (s->fd, fh, h, PDA_WRITE); > select_printf ("write: %s, n %d", fh->get_name (), n); > gotone += s->write_ready = (n > 0); > - if (n < 0 && s->except_selected) > + if (n == PDA_ERROR && s->except_selected) > gotone += s->except_ready = true; > } > return gotone; > -- > 2.45.1 > >
