Cleanup a little.

Use bit fields for flags to reduce memory usage.
Return booleans where they are documented.  Although a boolean for
seekable seems a bit odd when it can be unknown.  Add a comment about this.

diff --git a/Modules/_fileio.c b/Modules/_fileio.c
index abd5d19836aa94ab14658e85826f909b42e8f87b..2895b286bf5a535eeaba174678d3576e927be065 100644 (file)
--- a/Modules/_fileio.c
+++ b/Modules/_fileio.c
@@ -30,9 +30,9 @@
 typedef struct {
        PyObject_HEAD
        int fd;
-       int readable;
-       int writable;
-       int seekable; /* -1 means unknown */
+       unsigned readable : 1;
+       unsigned writable : 1;
+       int seekable : 2; /* -1 means unknown */
        PyObject *weakreflist;
 } PyFileIOObject;
 
@@ -40,23 +40,29 @@ PyTypeObject PyFileIO_Type;
 
 #define PyFileIO_Check(op) (PyObject_TypeCheck((op), &PyFileIO_Type))
 
-/* Note: if this function is changed so that it can return a true value,
- * then we need a separate function for __exit__
- */
-static PyObject *
-fileio_close(PyFileIOObject *self)
+/* Returns 0 on success, errno (which is < 0) on failure. */
+static int
+internal_close(PyFileIOObject *self)
 {
+       int save_errno = 0;
        if (self->fd >= 0) {
                int fd = self->fd;
                self->fd = -1;
                Py_BEGIN_ALLOW_THREADS
-               errno = 0;
-               close(fd);
+               if (close(fd) < 0)
+                       save_errno = errno;
                Py_END_ALLOW_THREADS
-               if (errno < 0) {
-                       PyErr_SetFromErrno(PyExc_IOError);
-                       return NULL;
-               }
+       }
+       return save_errno;
+}
+
+static PyObject *
+fileio_close(PyFileIOObject *self)
+{
+       errno = internal_close(self);
+       if (errno < 0) {
+               PyErr_SetFromErrno(PyExc_IOError);
+               return NULL;
        }
 
        Py_RETURN_NONE;
@@ -96,8 +102,7 @@ dircheck(PyFileIOObject* self)
                char *msg = "Is a directory";
 #endif
                PyObject *exc;
-               PyObject *closeresult = fileio_close(self);
-               Py_DECREF(closeresult);
+               internal_close(self);
 
                exc = PyObject_CallFunction(PyExc_IOError, "(is)",
                                            EISDIR, msg);
@@ -127,13 +132,10 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds)
        int fd = -1;
 
        assert(PyFileIO_Check(oself));
-       if (self->fd >= 0)
-       {
+       if (self->fd >= 0) {
                /* Have to close the existing file first. */
-               PyObject *closeresult = fileio_close(self);
-               if (closeresult == NULL)
+               if (internal_close(self) < 0)
                        return -1;
-               Py_DECREF(closeresult);
        }
 
        if (PyArg_ParseTupleAndKeywords(args, kwds, "i|s:fileio",
@@ -241,10 +243,10 @@ fileio_init(PyObject *oself, PyObject *args, PyObject *kwds)
                errno = 0;
 #ifdef MS_WINDOWS
                if (widename != NULL)
-               self->fd = _wopen(widename, flags, 0666);
+                       self->fd = _wopen(widename, flags, 0666);
                else
 #endif
-               self->fd = open(name, flags, 0666);
+                       self->fd = open(name, flags, 0666);
                Py_END_ALLOW_THREADS
                if (self->fd < 0 || dircheck(self) < 0) {
                        PyErr_SetFromErrnoWithFilename(PyExc_IOError, name);
@@ -269,16 +271,15 @@ fileio_dealloc(PyFileIOObject *self)
                PyObject_ClearWeakRefs((PyObject *) self);
 
        if (self->fd >= 0) {
-               PyObject *closeresult = fileio_close(self);
-               if (closeresult == NULL) {
+               errno = internal_close(self);
+               if (errno < 0) {
 #ifdef HAVE_STRERROR
                        PySys_WriteStderr("close failed: [Errno %d] %s\n",
                                           errno, strerror(errno));
 #else
                        PySys_WriteStderr("close failed: [Errno %d]\n", errno);
 #endif
-               } else
-                       Py_DECREF(closeresult);
+               }
        }
 
        Py_Type(self)->tp_free((PyObject *)self);
@@ -311,7 +312,7 @@ fileio_readable(PyFileIOObject *self)
 {
        if (self->fd < 0)
                return err_closed();
-       return PyInt_FromLong((long) self->readable);
+       return PyBool_FromLong((long) self->readable);
 }
 
 static PyObject *
@@ -319,7 +320,7 @@ fileio_writable(PyFileIOObject *self)
 {
        if (self->fd < 0)
                return err_closed();
-       return PyInt_FromLong((long) self->writable);
+       return PyBool_FromLong((long) self->writable);
 }
 
 static PyObject *
@@ -337,6 +338,8 @@ fileio_seekable(PyFileIOObject *self)
                else
                        self->seekable = 1;
        }
+       /* XXX(nnorwitz): should this return an int rather than a bool,
+          since seekable could be -1, 0, or 1? */
        return PyBool_FromLong((long) self->seekable);
 }
 
@@ -365,7 +368,7 @@ fileio_readinto(PyFileIOObject *self, PyObject *args)
                return NULL;
        }
 
-       return PyInt_FromLong(n);
+       return PyInt_FromSsize_t(n);
 }
 
 #define DEFAULT_BUFFER_SIZE (8*1024)
@@ -498,7 +501,7 @@ fileio_write(PyFileIOObject *self, PyObject *args)
                return NULL;
        }
 
-       return PyInt_FromLong(n);
+       return PyInt_FromSsize_t(n);
 }
 
 /* XXX Windows support below is likely incomplete */