openstack/deb-python-pygit2
Code Issues Proposed changes

Make dealloc use tp_free or don't allow inheritance

Browse Source 
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.
This commit is contained in:
Carlos Martín Nieto 2014-03-27 09:45:10 +01:00
parent e4d45118e6
commit 940a6da929
14 changed files with 20 additions and 24 deletions

4
TODO.txt

@ -14,7 +14,3 @@ Other
- Make the Py_LOCAL_INLINE macro to work with Python 2.6, 2.7 and 3.1 - 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 - Use surrogateescape in Python 3, see PEP-383
- Expose the ODB (Repository.odb) - 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.

6
src/blame.c

@ -170,7 +170,7 @@ PyTypeObject BlameHunkType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
BlameHunk__doc__, /* tp_doc */ BlameHunk__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -233,7 +233,7 @@ PyTypeObject BlameIterType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
BlameIter__doc__, /* tp_doc */ BlameIter__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -362,7 +362,7 @@ PyTypeObject BlameType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Blame__doc__, /* tp_doc */ Blame__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/blob.c

@ -181,7 +181,7 @@ PyTypeObject BlobType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Blob__doc__, /* tp_doc */ Blob__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/commit.c

@ -259,7 +259,7 @@ PyTypeObject CommitType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Commit__doc__, /* tp_doc */ Commit__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/config.c

@ -92,7 +92,7 @@ void
Config_dealloc(Config *self) Config_dealloc(Config *self)
{ {
git_config_free(self->config); git_config_free(self->config);
PyObject_Del(self); Py_TYPE(self)->tp_free(self);
} }
PyDoc_STRVAR(Config_get_global_config__doc__, PyDoc_STRVAR(Config_get_global_config__doc__,

4
src/diff.c

@ -207,7 +207,7 @@ PyTypeObject PatchType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Patch__doc__, /* tp_doc */ Patch__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -269,7 +269,7 @@ PyTypeObject DiffIterType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
DiffIter__doc__, /* tp_doc */ DiffIter__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

4
src/index.c

@ -71,7 +71,7 @@ Index_dealloc(Index* self)
PyObject_GC_UnTrack(self); PyObject_GC_UnTrack(self);
Py_XDECREF(self->repo); Py_XDECREF(self->repo);
git_index_free(self->index); git_index_free(self->index);
PyObject_GC_Del(self); Py_TYPE(self)->tp_free(self);
} }
int int
@ -561,7 +561,7 @@ void
IndexIter_dealloc(IndexIter *self) IndexIter_dealloc(IndexIter *self)
{ {
Py_CLEAR(self->owner); Py_CLEAR(self->owner);
PyObject_Del(self); Py_TYPE(self)->tp_free(self);
} }
PyObject * PyObject *

4
src/note.c

@ -134,7 +134,7 @@ PyTypeObject NoteType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Note__doc__, /* tp_doc */ Note__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -200,7 +200,7 @@ PyTypeObject NoteIterType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
NoteIter__doc__, /* tp_doc */ NoteIter__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/object.c

@ -45,7 +45,7 @@ Object_dealloc(Object* self)
{ {
Py_CLEAR(self->repo); Py_CLEAR(self->repo);
git_object_free(self->obj); git_object_free(self->obj);
PyObject_Del(self); Py_TYPE(self)->tp_free(self);
} }

2
src/reference.c

@ -97,7 +97,7 @@ PyTypeObject RefLogIterType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
RefLogIterType__doc__, /* tp_doc */ RefLogIterType__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/remote.c

@ -658,7 +658,7 @@ PyTypeObject RemoteType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Remote__doc__, /* tp_doc */ Remote__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

2
src/repository.c

@ -105,7 +105,7 @@ Repository_dealloc(Repository *self)
Py_CLEAR(self->index); Py_CLEAR(self->index);
Py_CLEAR(self->config); Py_CLEAR(self->config);
git_repository_free(self->repo); git_repository_free(self->repo);
PyObject_GC_Del(self); Py_TYPE(self)->tp_free(self);
} }
int int

2
src/tag.c

@ -151,7 +151,7 @@ PyTypeObject TagType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Tag__doc__, /* tp_doc */ Tag__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */

6
src/tree.c

@ -170,7 +170,7 @@ PyTypeObject TreeEntryType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
TreeEntry__doc__, /* tp_doc */ TreeEntry__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -518,7 +518,7 @@ PyTypeObject TreeType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
Tree__doc__, /* tp_doc */ Tree__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */
@ -589,7 +589,7 @@ PyTypeObject TreeIterType = {
0, /* tp_getattro */ 0, /* tp_getattro */
0, /* tp_setattro */ 0, /* tp_setattro */
0, /* tp_as_buffer */ 0, /* tp_as_buffer */
Py_TPFLAGS_DEFAULT | Py_TPFLAGS_BASETYPE, /* tp_flags */ Py_TPFLAGS_DEFAULT, /* tp_flags */
TreeIter__doc__, /* tp_doc */ TreeIter__doc__, /* tp_doc */
0, /* tp_traverse */ 0, /* tp_traverse */
0, /* tp_clear */ 0, /* tp_clear */