[Python-checkins] cpython: Issue #21679: Prevent extraneous fstat() calls during open(). Patch by

[antoine.pitrou]

http://hg.python.org/cpython/rev/3b5279b5bfd1
changeset:   91470:3b5279b5bfd1
user:        Antoine Pitrou <solipsis at pitrou.net>
date:        Sun Jun 29 20:07:28 2014 -0400
summary:
  Issue #21679: Prevent extraneous fstat() calls during open().  Patch by Bohuslav Kabrda.

files:
  Lib/test/test_fileio.py |  11 ++++++-
  Misc/NEWS               |   3 +
  Modules/_io/_iomodule.c |  28 +++++-----------
  Modules/_io/fileio.c    |  46 +++++++++++++++-------------
  4 files changed, 47 insertions(+), 41 deletions(-)


diff --git a/Lib/test/test_fileio.py b/Lib/test/test_fileio.py
--- a/Lib/test/test_fileio.py
+++ b/Lib/test/test_fileio.py
@@ -60,6 +60,15 @@
             self.assertRaises((AttributeError, TypeError),
                               setattr, f, attr, 'oops')
 
+    def testBlksize(self):
+        # test private _blksize attribute
+        blksize = io.DEFAULT_BUFFER_SIZE
+        # try to get preferred blksize from stat.st_blksize, if available
+        if hasattr(os, 'fstat'):
+            fst = os.fstat(self.f.fileno())
+            blksize = getattr(fst, 'st_blksize', blksize)
+        self.assertEqual(self.f._blksize, blksize)
+
     def testReadinto(self):
         # verify readinto
         self.f.write(bytes([1, 2]))
@@ -141,7 +150,7 @@
     def testOpendir(self):
         # Issue 3703: opening a directory should fill the errno
         # Windows always returns "[Errno 13]: Permission denied
-        # Unix calls dircheck() and returns "[Errno 21]: Is a directory"
+        # Unix uses fstat and returns "[Errno 21]: Is a directory"
         try:
             _FileIO('.', 'r')
         except OSError as e:
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -103,6 +103,9 @@
 Library
 -------
 
+- Issue #21679: Prevent extraneous fstat() calls during open().  Patch by
+  Bohuslav Kabrda.
+
 - Issue #21863: cProfile now displays the module name of C extension functions,
   in addition to their own name.
 
diff --git a/Modules/_io/_iomodule.c b/Modules/_io/_iomodule.c
--- a/Modules/_io/_iomodule.c
+++ b/Modules/_io/_iomodule.c
@@ -237,8 +237,8 @@
 
     PyObject *raw, *modeobj = NULL, *buffer, *wrapper, *result = NULL;
 
+    _Py_IDENTIFIER(_blksize);
     _Py_IDENTIFIER(isatty);
-    _Py_IDENTIFIER(fileno);
     _Py_IDENTIFIER(mode);
     _Py_IDENTIFIER(close);
 
@@ -380,24 +380,14 @@
         line_buffering = 0;
 
     if (buffering < 0) {
-        buffering = DEFAULT_BUFFER_SIZE;
-#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
-        {
-            struct stat st;
-            long fileno;
-            PyObject *res = _PyObject_CallMethodId(raw, &PyId_fileno, NULL);
-            if (res == NULL)
-                goto error;
-
-            fileno = PyLong_AsLong(res);
-            Py_DECREF(res);
-            if (fileno == -1 && PyErr_Occurred())
-                goto error;
-
-            if (fstat(fileno, &st) >= 0 && st.st_blksize > 1)
-                buffering = st.st_blksize;
-        }
-#endif
+        PyObject *blksize_obj;
+        blksize_obj = _PyObject_GetAttrId(raw, &PyId__blksize);
+        if (blksize_obj == NULL)
+            goto error;
+        buffering = PyLong_AsLong(blksize_obj);
+        Py_DECREF(blksize_obj);
+        if (buffering == -1 && PyErr_Occurred())
+            goto error;
     }
     if (buffering < 0) {
         PyErr_SetString(PyExc_ValueError,
diff --git a/Modules/_io/fileio.c b/Modules/_io/fileio.c
--- a/Modules/_io/fileio.c
+++ b/Modules/_io/fileio.c
@@ -53,6 +53,7 @@
     signed int seekable : 2; /* -1 means unknown */
     unsigned int closefd : 1;
     char finalizing;
+    unsigned int blksize;
     PyObject *weakreflist;
     PyObject *dict;
 } fileio;
@@ -161,6 +162,7 @@
         self->writable = 0;
         self->appending = 0;
         self->seekable = -1;
+        self->blksize = 0;
         self->closefd = 1;
         self->weakreflist = NULL;
     }
@@ -168,26 +170,6 @@
     return (PyObject *) self;
 }
 
-/* On Unix, open will succeed for directories.
-   In Python, there should be no file objects referring to
-   directories, so we need a check.  */
-
-static int
-dircheck(fileio* self, PyObject *nameobj)
-{
-#if defined(HAVE_FSTAT) && defined(S_ISDIR) && defined(EISDIR)
-    struct stat buf;
-    if (self->fd < 0)
-        return 0;
-    if (fstat(self->fd, &buf) == 0 && S_ISDIR(buf.st_mode)) {
-        errno = EISDIR;
-        PyErr_SetFromErrnoWithFilenameObject(PyExc_IOError, nameobj);
-        return -1;
-    }
-#endif
-    return 0;
-}
-
 static int
 check_fd(int fd)
 {
@@ -233,6 +215,9 @@
 #elif !defined(MS_WINDOWS)
     int *atomic_flag_works = NULL;
 #endif
+#ifdef HAVE_FSTAT
+    struct stat fdfstat;
+#endif
 
     assert(PyFileIO_Check(oself));
     if (self->fd >= 0) {
@@ -421,8 +406,26 @@
             goto error;
 #endif
     }
-    if (dircheck(self, nameobj) < 0)
+
+    self->blksize = DEFAULT_BUFFER_SIZE;
+#ifdef HAVE_FSTAT
+    if (fstat(self->fd, &fdfstat) < 0)
         goto error;
+#if defined(S_ISDIR) && defined(EISDIR)
+    /* On Unix, open will succeed for directories.
+       In Python, there should be no file objects referring to
+       directories, so we need a check.  */
+    if (S_ISDIR(fdfstat.st_mode)) {
+        errno = EISDIR;
+        PyErr_SetFromErrnoWithFilenameObject(PyExc_IOError, nameobj);
+        goto error;
+    }
+#endif /* defined(S_ISDIR) */
+#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
+    if (fdfstat.st_blksize > 1)
+        self->blksize = fdfstat.st_blksize;
+#endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */
+#endif /* HAVE_FSTAT */
 
 #if defined(MS_WINDOWS) || defined(__CYGWIN__)
     /* don't translate newlines (\r\n <=> \n) */
@@ -1216,6 +1219,7 @@
 };
 
 static PyMemberDef fileio_members[] = {
+    {"_blksize", T_UINT, offsetof(fileio, blksize), 0},
     {"_finalizing", T_BOOL, offsetof(fileio, finalizing), 0},
     {NULL}
 };

-- 
Repository URL: http://hg.python.org/cpython