From 940a6da92987acaba9954890be00e14cc71bbd4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= <cmn@dwim.me> Date: Thu, 27 Mar 2014 09:45:10 +0100 Subject: [PATCH] Make dealloc use tp_free or don't allow inheritance When a class is a base type, it must use its type's tp_free function to trigger the real free, instead of PyObjec_Del(). We do not always follow this convention, so let's give it a once-over and make sure we do that or that it's not a base type. Many of the types have the flag set in the struct, but do not pass the allocator function at init time, which makes them not really be a base. Remove the flag for those types. --- TODO.txt | 4 ---- src/blame.c | 6 +++--- src/blob.c | 2 +- src/commit.c | 2 +- src/config.c | 2 +- src/diff.c | 4 ++-- src/index.c | 4 ++-- src/note.c | 4 ++-- src/object.c | 2 +- src/reference.c | 2 +- src/remote.c | 2 +- src/repository.c | 2 +- src/tag.c | 2 +- src/tree.c | 6 +++--- 14 files changed, 20 insertions(+), 24 deletions(-) diff --git a/TODO.txt b/TODO.txt index 4a48a28..a648a06 100644 --- a/TODO.txt +++ b/TODO.txt @@ -14,7 +14,3 @@ Other - Make the Py_LOCAL_INLINE macro to work with Python 2.6, 2.7 and 3.1 - Use surrogateescape in Python 3, see PEP-383 - Expose the ODB (Repository.odb) -- According to Python documentation, tp_dealloc must call tp_free (instead of - PyObject_Del or similar) if the type is subclassable. So, go through the - code and switch to tp_free, or make the type not subclassable, on a case by - case basis. diff --git a/src/blame.c b/src/blame.c index f0ee18f..80b8208 100644 --- a/src/blame.c +++ b/src/blame.c @@ -170,7 +170,7 @@ PyTypeObject BlameHunkType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ BlameHunk__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -233,7 +233,7 @@ PyTypeObject BlameIterType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ BlameIter__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -362,7 +362,7 @@ PyTypeObject BlameType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Blame__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/blob.c b/src/blob.c index 94352df..133d494 100644 --- a/src/blob.c +++ b/src/blob.c @@ -181,7 +181,7 @@ PyTypeObject BlobType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Blob__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/commit.c b/src/commit.c index 8a61d29..85f59cd 100644 --- a/src/commit.c +++ b/src/commit.c @@ -259,7 +259,7 @@ PyTypeObject CommitType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Commit__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/config.c b/src/config.c index 8cbb054..9b2625d 100644 --- a/src/config.c +++ b/src/config.c @@ -92,7 +92,7 @@ void Config_dealloc(Config *self) { git_config_free(self->config); - PyObject_Del(self); + Py_TYPE(self)->tp_free(self); } PyDoc_STRVAR(Config_get_global_config__doc__, diff --git a/src/diff.c b/src/diff.c index b3a32dd..adea3cd 100644 --- a/src/diff.c +++ b/src/diff.c @@ -207,7 +207,7 @@ PyTypeObject PatchType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Patch__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -269,7 +269,7 @@ PyTypeObject DiffIterType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ DiffIter__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/index.c b/src/index.c index cc1b365..9fb5878 100644 --- a/src/index.c +++ b/src/index.c @@ -71,7 +71,7 @@ Index_dealloc(Index* self) PyObject_GC_UnTrack(self); Py_XDECREF(self->repo); git_index_free(self->index); - PyObject_GC_Del(self); + Py_TYPE(self)->tp_free(self); } int @@ -561,7 +561,7 @@ void IndexIter_dealloc(IndexIter *self) { Py_CLEAR(self->owner); - PyObject_Del(self); + Py_TYPE(self)->tp_free(self); } PyObject * diff --git a/src/note.c b/src/note.c index 2e5c9b1..e14e308 100644 --- a/src/note.c +++ b/src/note.c @@ -134,7 +134,7 @@ PyTypeObject NoteType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Note__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -200,7 +200,7 @@ PyTypeObject NoteIterType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ NoteIter__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/object.c b/src/object.c index 74c3801..f737e8b 100644 --- a/src/object.c +++ b/src/object.c @@ -45,7 +45,7 @@ Object_dealloc(Object* self) { Py_CLEAR(self->repo); git_object_free(self->obj); - PyObject_Del(self); + Py_TYPE(self)->tp_free(self); } diff --git a/src/reference.c b/src/reference.c index 2efbca3..e597e35 100644 --- a/src/reference.c +++ b/src/reference.c @@ -97,7 +97,7 @@ PyTypeObject RefLogIterType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ RefLogIterType__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/remote.c b/src/remote.c index c738784..ab085c8 100644 --- a/src/remote.c +++ b/src/remote.c @@ -658,7 +658,7 @@ PyTypeObject RemoteType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Remote__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/repository.c b/src/repository.c index 031e6e0..d42264a 100644 --- a/src/repository.c +++ b/src/repository.c @@ -105,7 +105,7 @@ Repository_dealloc(Repository *self) Py_CLEAR(self->index); Py_CLEAR(self->config); git_repository_free(self->repo); - PyObject_GC_Del(self); + Py_TYPE(self)->tp_free(self); } int diff --git a/src/tag.c b/src/tag.c index 6224d4c..d6507ae 100644 --- a/src/tag.c +++ b/src/tag.c @@ -151,7 +151,7 @@ PyTypeObject TagType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Tag__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ diff --git a/src/tree.c b/src/tree.c index 8c83d3a..d1968fa 100644 --- a/src/tree.c +++ b/src/tree.c @@ -170,7 +170,7 @@ PyTypeObject TreeEntryType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ TreeEntry__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -518,7 +518,7 @@ PyTypeObject TreeType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ Tree__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */ @@ -589,7 +589,7 @@ PyTypeObject TreeIterType = { 0, /* tp_getattro */ 0, /* tp_setattro */ 0, /* tp_as_buffer */ - Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ + Py_TPFLAGS_DEFAULT, /* tp_flags */ TreeIter__doc__, /* tp_doc */ 0, /* tp_traverse */ 0, /* tp_clear */