[Python-checkins] r79937 - in python/trunk: Lib/test/test_io.py Lib/test/test_posix.py Modules/_io/fileio.c

[antoine.pitrou]

Author: antoine.pitrou
Date: Sun Apr 11 00:43:05 2010
New Revision: 79937

Log:
Temporary commit of fix to issue #5380 (in order to watch buildbot response)



Modified:
   python/trunk/Lib/test/test_io.py
   python/trunk/Lib/test/test_posix.py
   python/trunk/Modules/_io/fileio.c

Modified: python/trunk/Lib/test/test_io.py
==============================================================================
--- python/trunk/Lib/test/test_io.py	(original)
+++ python/trunk/Lib/test/test_io.py	Sun Apr 11 00:43:05 2010
@@ -2394,6 +2394,47 @@
         # baseline "io" module.
         self._check_abc_inheritance(io)
 
+    # Issue #5380: reading all available bytes from a pipe or a PTY when
+    # the other end has been closed.
+
+    def check_pipe_func(self, pipe_func, buffered):
+        master_fd, slave_fd = pipe_func()
+        # Simulate a subprocess writing some data to the
+        # slave end of the pipe, and then exiting.
+        data = b'TEST DATA'
+        try:
+            os.write(slave_fd, data)
+        finally:
+            os.close(slave_fd)
+        with self.open(master_fd, "rb", buffering=-1 if buffered else 0) as f:
+            # Request more data than available
+            gotdata = f.read(len(data) + 1)
+            self.assertEqual(gotdata, data)
+            # Trying to read again returns an empty string
+            self.assertEqual(b'', f.read())
+            self.assertEqual(b'', f.read(1))
+
+    def test_pipe_read_buffered(self):
+        if not hasattr(os, 'pipe'):
+            self.skipTest("os.pipe not available")
+        self.check_pipe_func(os.pipe, True)
+
+    def test_pipe_read_raw(self):
+        if not hasattr(os, 'pipe'):
+            self.skipTest("os.pipe not available")
+        self.check_pipe_func(os.pipe, False)
+
+    def test_openpty_read_buffered(self):
+        if not hasattr(os, 'openpty'):
+            self.skipTest("os.openpty not available")
+        self.check_pipe_func(os.openpty, True)
+
+    def test_openpty_read_raw(self):
+        if not hasattr(os, 'openpty'):
+            self.skipTest("os.openpty not available")
+        self.check_pipe_func(os.openpty, False)
+
+
 class CMiscIOTest(MiscIOTest):
     io = io
 

Modified: python/trunk/Lib/test/test_posix.py
==============================================================================
--- python/trunk/Lib/test/test_posix.py	(original)
+++ python/trunk/Lib/test/test_posix.py	Sun Apr 11 00:43:05 2010
@@ -280,11 +280,29 @@
         if hasattr(posix, 'strerror'):
             self.assertTrue(posix.strerror(0))
 
+    def check_pipe_func(self, pipe_func):
+        master_fd, slave_fd = pipe_func()
+        try:
+            # Simulate a subprocess writing some data to the
+            # slave end of the pipe, and then exiting.
+            data = b'TEST DATA'
+            try:
+                os.write(slave_fd, data)
+            finally:
+                os.close(slave_fd)
+            # Request more data than available
+            gotdata = os.read(master_fd, len(data) + 1)
+            self.assertEqual(gotdata, data)
+        finally:
+            os.close(master_fd)
+
     def test_pipe(self):
         if hasattr(posix, 'pipe'):
-            reader, writer = posix.pipe()
-            os.close(reader)
-            os.close(writer)
+            self.check_pipe_func(posix.pipe)
+
+    def test_openpty(self):
+        if hasattr(posix, 'openpty'):
+            self.check_pipe_func(posix.openpty)
 
     def test_tempnam(self):
         if hasattr(posix, 'tempnam'):

Modified: python/trunk/Modules/_io/fileio.c
==============================================================================
--- python/trunk/Modules/_io/fileio.c	(original)
+++ python/trunk/Modules/_io/fileio.c	Sun Apr 11 00:43:05 2010
@@ -464,6 +464,34 @@
 	return PyBool_FromLong((long) self->seekable);
 }
 
+static Py_ssize_t
+internal_read(int fd, void *buf, size_t count)
+{
+	Py_ssize_t n;
+
+	Py_BEGIN_ALLOW_THREADS
+	errno = 0;
+	n = read(fd, buf, count);
+#ifdef EIO
+	/* Issue #5380: when reading past the end of a pipe created by
+	   openpty(), EIO can be set.  Make it an EOF instead, so that
+	   the normal technique of testing for an empty string can be used.
+	 */
+	if (n == -1 && errno == EIO) {
+		if (isatty(fd)) {
+			n = 0;
+			errno = 0;
+		}
+		else {
+			/* isatty() set errno, restore its value */
+			errno = EIO;
+		}
+	}
+#endif
+	Py_END_ALLOW_THREADS
+	return n;
+}
+
 static PyObject *
 fileio_readinto(fileio *self, PyObject *args)
 {
@@ -478,12 +506,9 @@
 	if (!PyArg_ParseTuple(args, "w*", &pbuf))
 		return NULL;
 
-	if (_PyVerify_fd(self->fd)) {
-		Py_BEGIN_ALLOW_THREADS
-		errno = 0;
-		n = read(self->fd, pbuf.buf, pbuf.len);
-		Py_END_ALLOW_THREADS
-	} else
+	if (_PyVerify_fd(self->fd))
+		n = internal_read(self->fd, pbuf.buf, pbuf.len);
+	else
 		n = -1;
 	PyBuffer_Release(&pbuf);
 	if (n < 0) {
@@ -560,12 +585,9 @@
 				break;
 			}
 		}
-		Py_BEGIN_ALLOW_THREADS
-		errno = 0;
-		n = read(self->fd,
-			 PyBytes_AS_STRING(result) + total,
-			 newsize - total);
-		Py_END_ALLOW_THREADS
+		n = internal_read(self->fd,
+				  PyBytes_AS_STRING(result) + total,
+				  newsize - total);
 		if (n == 0)
 			break;
 		if (n < 0) {
@@ -617,12 +639,9 @@
 		return NULL;
 	ptr = PyBytes_AS_STRING(bytes);
 
-	if (_PyVerify_fd(self->fd)) {
-		Py_BEGIN_ALLOW_THREADS
-		errno = 0;
-		n = read(self->fd, ptr, size);
-		Py_END_ALLOW_THREADS
-	} else
+	if (_PyVerify_fd(self->fd))
+		n = internal_read(self->fd, ptr, size);
+	else
 		n = -1;
 
 	if (n < 0) {