The end of [#460467] file objects should be subclassable.
A surprising number of changes to split tp_new into tp_new and tp_init.
Turned out the older PyFile_FromFile() didn't initialize the memory it
allocated in all (error) cases, which caused new sanity asserts
elsewhere to fail left & right (and could have, e.g., caused file_dealloc
to try decrefing random addresses).
diff --git a/Objects/fileobject.c b/Objects/fileobject.c
index 578b6f0..779b5fa 100644
--- a/Objects/fileobject.c
+++ b/Objects/fileobject.c
@@ -72,12 +72,17 @@
{
assert(f != NULL);
assert(PyFile_Check(f));
- f->f_fp = NULL;
+ assert(f->f_fp == NULL);
+
+ Py_DECREF(f->f_name);
+ Py_DECREF(f->f_mode);
f->f_name = PyString_FromString(name);
f->f_mode = PyString_FromString(mode);
+
f->f_close = close;
f->f_softspace = 0;
f->f_binary = strchr(mode,'b') != NULL;
+
if (f->f_name == NULL || f->f_mode == NULL)
return NULL;
f->f_fp = fp;
@@ -91,6 +96,7 @@
assert(PyFile_Check(f));
assert(name != NULL);
assert(mode != NULL);
+ assert(f->f_fp == NULL);
/* rexec.py can't stop a user from getting the file() constructor --
all they have to do is get *any* file object f, and then do
@@ -130,7 +136,8 @@
PyObject *
PyFile_FromFile(FILE *fp, char *name, char *mode, int (*close)(FILE *))
{
- PyFileObject *f = PyObject_NEW(PyFileObject, &PyFile_Type);
+ PyFileObject *f = (PyFileObject *)PyFile_Type.tp_new(&PyFile_Type,
+ NULL, NULL);
if (f != NULL) {
if (fill_file_fields(f, fp, name, mode, close) == NULL) {
Py_DECREF(f);
@@ -199,12 +206,8 @@
(*f->f_close)(f->f_fp);
Py_END_ALLOW_THREADS
}
- if (f->f_name != NULL) {
- Py_DECREF(f->f_name);
- }
- if (f->f_mode != NULL) {
- Py_DECREF(f->f_mode);
- }
+ Py_XDECREF(f->f_name);
+ Py_XDECREF(f->f_mode);
PyObject_DEL(f);
}
@@ -1328,35 +1331,65 @@
static PyObject *
file_new(PyTypeObject *type, PyObject *args, PyObject *kwds)
{
- /* XXX As for all XXX_new functions, file_new is called
- with kwds=NULL by type_call(), so the kwlist is impotent. */
+ PyObject *self;
+ static PyObject *not_yet_string;
+
+ assert(type != NULL && type->tp_alloc != NULL);
+
+ if (not_yet_string == NULL) {
+ not_yet_string = PyString_FromString("<uninitialized file>");
+ if (not_yet_string == NULL)
+ return NULL;
+ }
+
+ self = type->tp_alloc(type, 0);
+ if (self != NULL) {
+ /* Always fill in the name and mode, so that nobody else
+ needs to special-case NULLs there. */
+ Py_INCREF(not_yet_string);
+ ((PyFileObject *)self)->f_name = not_yet_string;
+ Py_INCREF(not_yet_string);
+ ((PyFileObject *)self)->f_mode = not_yet_string;
+ }
+ return self;
+}
+
+static int
+file_init(PyObject *self, PyObject *args, PyObject *kwds)
+{
+ PyFileObject *foself = (PyFileObject *)self;
+ int ret = 0;
static char *kwlist[] = {"name", "mode", "buffering", 0};
char *name = NULL;
char *mode = "r";
int bufsize = -1;
- PyObject *f;
- extern int fclose(FILE *);
+
+ assert(PyFile_Check(self));
+ if (foself->f_fp != NULL) {
+ /* Have to close the existing file first. */
+ PyObject *closeresult = file_close(foself);
+ if (closeresult == NULL)
+ return -1;
+ Py_DECREF(closeresult);
+ }
if (!PyArg_ParseTupleAndKeywords(args, kwds, "et|si:file", kwlist,
Py_FileSystemDefaultEncoding, &name,
&mode, &bufsize))
- return NULL;
- f = PyType_GenericAlloc(type, 0);
- if (f != NULL) {
- PyFileObject *g = (PyFileObject *)f;
- if (fill_file_fields(g, NULL, name, mode, fclose) == NULL) {
- Py_DECREF(f);
- f = NULL;
- }
- if (f != NULL && open_the_file(g, name, mode) == NULL) {
- Py_DECREF(f);
- f = NULL;
- }
- if (f != NULL)
- PyFile_SetBufSize(f, bufsize);
- }
+ return -1;
+ if (fill_file_fields(foself, NULL, name, mode, fclose) == NULL)
+ goto Error;
+ if (open_the_file(foself, name, mode) == NULL)
+ goto Error;
+ PyFile_SetBufSize(self, bufsize);
+ goto Done;
+
+Error:
+ ret = -1;
+ /* fall through */
+Done:
PyMem_Free(name); /* free the encoded string */
- return f;
+ return ret;
}
static char file_doc[] =
@@ -1409,8 +1442,8 @@
0, /* tp_descr_get */
0, /* tp_descr_set */
0, /* tp_dictoffset */
- 0, /* tp_init */
- 0, /* tp_alloc */
+ (initproc)file_init, /* tp_init */
+ PyType_GenericAlloc, /* tp_alloc */
file_new, /* tp_new */
};