coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed ou


From: Carl Edquist
Subject: Re: [PATCH] tee: Add --pipe-check to allow instantly detecting closed outputs
Date: Wed, 7 Dec 2022 17:22:40 -0600 (CST)

Alright, lest I be guilty of idle nay-saying, I've attached another patch to address all of my complaints.

(Apply it after Arsen's last one, which comes after my previous one. Otherwise if desired I can send a single summary patch.)

Biggest item is making a new configure macro based on whether poll() is present and and works as intended for pipes. With 0 timeout, polling the write-end of a pipe that is open on both ends for errors does not indicate a broken pipe; but polling the write-end of a pipe with the read-end closed does indicate a broken pipe.

Hopefully this test covers all platforms. The only part that I'm iffy about was this bit that Pádraig mentioned:

Portability problems fixed by Gnulib:

 This function doesn't work on special files like @file{/dev/null} and
 ttys like @file{/dev/tty} on some platforms: AIX 5.3, Mac OS X 10.4.0

If indeed there are issues on these platforms using poll() against these special files (character devices), I suspect we can just test the relevant behavior in the test program for the new configure macro. (eg, poll() for /dev/null should always show POLLIN when open for reading, and should not returning any errors when open for writing.)


Beyond that, I revised the select()-based implementation of iopoll() to address my previous comments. Sorry I got my grubby hands all over it.
I do hope you'll like it though! :)


Cheers,
Carl


On Tue, 6 Dec 2022, Carl Edquist wrote:

 (4.)

  +  /* readable event on STDOUT is equivalent to POLLERR,
  +     and implies an error condition on output like broken pipe.  */

 I know this is what the comment from tail.c says, but is it actually
 documented to be true somewhere?  And on what platforms?  I don't see it
 being documented in my select(2) on Linux, anyway.  (Though it does seem
 to work.)  Wondering if this behavior is "standard".

Ah!

Well, it's not documented in my (oldish) select(2), but I do find the following in a newer version of that manpage:


 Correspondence between select() and poll() notifications

  Within the Linux kernel source, we find the following definitions which
  show the correspondence between the readable, writable, and exceptional
  condition  notifications  of  select() and the event notifications pro-
  vided by poll(2) (and epoll(7)):

      #define POLLIN_SET (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP |
                         POLLERR)
                         /* Ready for reading */
      #define POLLOUT_SET (POLLWRBAND | POLLWRNORM | POLLOUT | POLLERR)
                         /* Ready for writing */
      #define POLLEX_SET (POLLPRI)
                         /* Exceptional condition */



So I guess (on Linux at least) that means a "readable event on STDOUT" is equivalent to (POLLRDNORM | POLLRDBAND | POLLIN | POLLHUP | POLLERR).

So, it'll catch the relevant poll() errors (POLLHUP | POLLERR), but the inclusion of POLLIN results in the gotcha that it will be a false positive if stdout is already open for RW (eg a socket) and there is actually data ready.

Also, the POLLEX_SET definition of (POLLPRI) doesn't seem relevant; so I might suggest removing the 'xfd' arg for the select()-based implementation:

 POLLPRI

   There is some exceptional  condition  on  the  file  descriptor.
   Possibilities include:

   *  There is out-of-band data on a TCP socket (see tcp(7)).

   *  A  pseudoterminal  master  in  packet  mode  has seen a state
      change on the slave (see ioctl_tty(2)).

   *  A cgroup.events file has been modified (see cgroups(7)).


Of course, those definitions live in the Linux kernel source, and on Linux we'd get to use poll() instead of select().

Don't know if the select() <-> poll() correspondence differs at all on other platforms .. ?


(But, this does give me a bit more confidence about select() being a (mostly) viable alternative.)


Carl

Attachment: 0001-tee-define-and-use-HAVE_POLL_PIPE_CLOSE_DETECTION.patch
Description: Text Data


reply via email to

[Prev in Thread] Current Thread [Next in Thread]