[prev in list] [next in list] [prev in thread] [next in thread] 

List:       coreutils
Subject:    Re: [PATCH] tee: recover from EAGAIN returned from fwrite()
From:       Kamil Dudka <kdudka () redhat ! com>
Date:       2018-09-11 7:50:19
Message-ID: 4691894.mg1HmZHfco () kdudka-nb
[Download RAW message or body]

On Tuesday, September 11, 2018 6:49:09 AM CEST Pádraig Brady wrote:
> On 10/09/18 09:38, Kamil Dudka wrote:
> > On Friday, September 7, 2018 5:21:04 AM CEST Pádraig Brady wrote:
> >> Also would `cat | telnet ... | tee blah` be a workaround?
> > 
> > The above command does not really work because telnet is unable to set all
> > the other needed properties on the terminal.  So it let me write my
> > password on the screen (because 'echo' was not disabled), arrow keys in
> > my shell session did not work, etc.
> 
> Right. However if telnet wants low level control of the terminal
> and tee is unsetting fundamental IO behaviors of the tty like this,
> then it wouldn't be general for all uses of telnet (like connecting
> to a non line oriented remote session for example).

Fair point.  What about the attached patch instead?

Kamil

> I.E. this isn't a general solution for all coreutils, or even for tee.
> 
> Options:
> 
> If one wants simple line oriented connection use netcat or equivalent.
> 
> Use script(1) which is more suited to recording terminal sessions.
> 
> Adjust telnet to not use FIONBIO when in line mode.
> 
> Avoid the tee apparent hang by using the --output-error=exit option.
> 
> cheers,
> Pádraig
["0001-tee-handle-EAGAIN-returned-from-fwrite.patch" (0001-tee-handle-EAGAIN-returned-from-fwrite.patch)]

>From 8935a5cfde598e1eebfc857ebbf9563ecabe490e Mon Sep 17 00:00:00 2001
From: Kamil Dudka <kdudka@redhat.com>
Date: Thu, 16 Aug 2018 13:00:14 +0200
Subject: [PATCH] tee: handle EAGAIN returned from fwrite()

'tee' expected the output file descriptors to operate in blocking mode
but this assumption might be invalidated by other programs connected to
the same terminal, as in the following example:

$ telnet ... | tee log_file

telnet calls ioctl(stdin, FIONBIO, [1]), which causes the O_NONBLOCK
flag to be set on tee's output, which is connected to the same terminal.

This patch has zero impact unless EAGAIN returns from fwrite().  In that
case 'tee' waits for the underlying file descriptor to become writable
again and then it writes the remaining data.
---
 src/tee.c | 46 +++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 45 insertions(+), 1 deletion(-)

diff --git a/src/tee.c b/src/tee.c
index dae0f1e50..e657a12ec 100644
--- a/src/tee.c
+++ b/src/tee.c
@@ -17,6 +17,8 @@
 /* Mike Parker, Richard M. Stallman, and David MacKenzie */
 
 #include <config.h>
+#include <assert.h>
+#include <poll.h>
 #include <sys/types.h>
 #include <signal.h>
 #include <getopt.h>
@@ -176,6 +178,48 @@ main (int argc, char **argv)
   return ok ? EXIT_SUCCESS : EXIT_FAILURE;
 }
 
+/* wrapper of fwrite() that handles EAGAIN properly in case the O_NONBLOCK
+   flag was set on the output file descriptor */
+static bool
+write_buf (const char *buf, ssize_t size, FILE *f)
+{
+  for (;;)
+    {
+      struct pollfd pfd;
+      const size_t written = fwrite (buf, 1, size, f);
+      size -= written;
+      assert (size >= 0);
+      if (size <= 0)
+        /* everything written */
+        return true;
+
+      if (errno != EAGAIN)
+        /* non-recoverable write error */
+        return false;
+
+      /* update position in the write buffer */
+      buf += written;
+
+      /* obtain file descriptor we are writing to */
+      pfd.fd = fileno (f);
+      if (pfd.fd == -1)
+        goto fail;
+
+      /* wait for the file descriptor to become writable */
+      pfd.events = POLLOUT;
+      pfd.revents = 0;
+      if (poll (&pfd, 1, -1) != 1)
+        goto fail;
+      if (!(pfd.revents & POLLOUT))
+        goto fail;
+    }
+
+fail:
+  /* preserve the original errno value in case we failed to handle EAGAIN */
+  errno = EAGAIN;
+  return false;
+}
+
 /* Copy the standard input into each of the NFILES files in FILES
    and into the standard output.  As a side effect, modify FILES[-1].
    Return true if successful.  */
@@ -238,7 +282,7 @@ tee_files (int nfiles, char **files)
          Standard output is the first one.  */
       for (i = 0; i <= nfiles; i++)
         if (descriptors[i]
-            && fwrite (buffer, bytes_read, 1, descriptors[i]) != 1)
+            && !write_buf (buffer, bytes_read, descriptors[i]))
           {
             int w_errno = errno;
             bool fail = errno != EPIPE || (output_error == output_error_exit
-- 
2.17.1



[prev in list] [next in list] [prev in thread] [next in thread]