From 7736a6b3d3453112890825fdf040744d61b026aa Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Fri, 12 Jun 2020 22:56:22 +0900 Subject: [PATCH 1/9] bpo-1635741: Port _dbm module to multiphase initialization --- ...2020-06-12-22-56-17.bpo-1635741.mmlp3Q.rst | 1 + Modules/_dbmmodule.c | 275 ++++++++++-------- Modules/clinic/_dbmmodule.c.h | 45 ++- 3 files changed, 187 insertions(+), 134 deletions(-) create mode 100644 Misc/NEWS.d/next/Core and Builtins/2020-06-12-22-56-17.bpo-1635741.mmlp3Q.rst diff --git a/Misc/NEWS.d/next/Core and Builtins/2020-06-12-22-56-17.bpo-1635741.mmlp3Q.rst b/Misc/NEWS.d/next/Core and Builtins/2020-06-12-22-56-17.bpo-1635741.mmlp3Q.rst new file mode 100644 index 00000000000000..ae12d25baa3ade --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2020-06-12-22-56-17.bpo-1635741.mmlp3Q.rst @@ -0,0 +1 @@ +Port :mod:`_dbm` to multiphase initialization. diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 80a0503622c3fe..11a6d86900d9f6 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -28,6 +28,19 @@ static const char which_dbm[] = "Berkeley DB"; #error "No ndbm.h available!" #endif +typedef struct { + PyTypeObject *dbm_type; + PyObject *dbm_error; +} _dbm_state; + +static inline _dbm_state* +get_dbm_state(PyObject *module) +{ + void *state = PyModule_GetState(module); + assert(state != NULL); + return (_dbm_state *)state; +} + /*[clinic input] module _dbm class _dbm.dbm "dbmobject *" "&Dbmtype" @@ -43,28 +56,23 @@ typedef struct { #include "clinic/_dbmmodule.c.h" -static PyTypeObject Dbmtype; - -#define is_dbmobject(v) Py_IS_TYPE(v, &Dbmtype) -#define check_dbmobject_open(v) if ((v)->di_dbm == NULL) \ - { PyErr_SetString(DbmError, "DBM object has already been closed"); \ +#define check_dbmobject_open(v, err) if ((v)->di_dbm == NULL) \ + { PyErr_SetString(err, "DBM object has already been closed"); \ return NULL; } -static PyObject *DbmError; - static PyObject * -newdbmobject(const char *file, int flags, int mode) +newdbmobject(_dbm_state *state, const char *file, int flags, int mode) { dbmobject *dp; - dp = PyObject_New(dbmobject, &Dbmtype); + dp = PyObject_New(dbmobject, state->dbm_type); if (dp == NULL) return NULL; dp->di_size = -1; dp->flags = flags; /* See issue #19296 */ if ( (dp->di_dbm = dbm_open((char *)file, flags, mode)) == 0 ) { - PyErr_SetFromErrnoWithFilename(DbmError, file); + PyErr_SetFromErrnoWithFilename(state->dbm_error, file); Py_DECREF(dp); return NULL; } @@ -78,14 +86,22 @@ dbm_dealloc(dbmobject *dp) { if ( dp->di_dbm ) dbm_close(dp->di_dbm); - PyObject_Del(dp); + PyTypeObject *tp = Py_TYPE(dp); + freefunc free_func = PyType_GetSlot(tp, Py_tp_free); + free_func(dp); + Py_DECREF(tp); } static Py_ssize_t dbm_length(dbmobject *dp) { + PyTypeObject *tp = Py_TYPE(dp); + _dbm_state *state = PyType_GetModuleState(tp); + if (state == NULL) { + return -1; + } if (dp->di_dbm == NULL) { - PyErr_SetString(DbmError, "DBM object has already been closed"); + PyErr_SetString(state->dbm_error, "DBM object has already been closed"); return -1; } if ( dp->di_size < 0 ) { @@ -106,12 +122,16 @@ dbm_subscript(dbmobject *dp, PyObject *key) { datum drec, krec; Py_ssize_t tmp_size; - + PyTypeObject *tp = Py_TYPE(dp); + _dbm_state *state = PyType_GetModuleState(tp); + if (state == NULL) { + return NULL; + } if (!PyArg_Parse(key, "s#", &krec.dptr, &tmp_size) ) return NULL; krec.dsize = tmp_size; - check_dbmobject_open(dp); + check_dbmobject_open(dp, state->dbm_error); drec = dbm_fetch(dp->di_dbm, krec); if ( drec.dptr == 0 ) { PyErr_SetObject(PyExc_KeyError, key); @@ -119,7 +139,7 @@ dbm_subscript(dbmobject *dp, PyObject *key) } if ( dbm_error(dp->di_dbm) ) { dbm_clearerr(dp->di_dbm); - PyErr_SetString(DbmError, ""); + PyErr_SetString(state->dbm_error, ""); return NULL; } return PyBytes_FromStringAndSize(drec.dptr, drec.dsize); @@ -136,9 +156,14 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w) "dbm mappings have bytes or string keys only"); return -1; } + PyTypeObject *tp = Py_TYPE(dp); + _dbm_state *state = PyType_GetModuleState(tp); + if (state == NULL) { + return -1; + } krec.dsize = tmp_size; if (dp->di_dbm == NULL) { - PyErr_SetString(DbmError, "DBM object has already been closed"); + PyErr_SetString(state->dbm_error, "DBM object has already been closed"); return -1; } dp->di_size = -1; @@ -151,7 +176,7 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w) PyErr_SetObject(PyExc_KeyError, v); } else { - PyErr_SetString(DbmError, "cannot delete item from database"); + PyErr_SetString(state->dbm_error, "cannot delete item from database"); } return -1; } @@ -164,25 +189,19 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w) drec.dsize = tmp_size; if ( dbm_store(dp->di_dbm, krec, drec, DBM_REPLACE) < 0 ) { dbm_clearerr(dp->di_dbm); - PyErr_SetString(DbmError, + PyErr_SetString(state->dbm_error, "cannot add item to database"); return -1; } } if ( dbm_error(dp->di_dbm) ) { dbm_clearerr(dp->di_dbm); - PyErr_SetString(DbmError, ""); + PyErr_SetString(state->dbm_error, ""); return -1; } return 0; } -static PyMappingMethods dbm_as_mapping = { - (lenfunc)dbm_length, /*mp_length*/ - (binaryfunc)dbm_subscript, /*mp_subscript*/ - (objobjargproc)dbm_ass_sub, /*mp_ass_subscript*/ -}; - /*[clinic input] _dbm.dbm.close @@ -202,18 +221,24 @@ _dbm_dbm_close_impl(dbmobject *self) /*[clinic input] _dbm.dbm.keys + cls: defining_class + Return a list of all keys in the database. [clinic start generated code]*/ static PyObject * -_dbm_dbm_keys_impl(dbmobject *self) -/*[clinic end generated code: output=434549f7c121b33c input=d210ba778cd9c68a]*/ +_dbm_dbm_keys_impl(dbmobject *self, PyTypeObject *cls) +/*[clinic end generated code: output=f2a593b3038e5996 input=d3706a28fc051097]*/ { PyObject *v, *item; datum key; int err; - check_dbmobject_open(self); + _dbm_state *state = PyType_GetModuleState(cls); + if (state == NULL) { + return NULL; + } + check_dbmobject_open(self, state->dbm_error); v = PyList_New(0); if (v == NULL) return NULL; @@ -241,8 +266,13 @@ dbm_contains(PyObject *self, PyObject *arg) datum key, val; Py_ssize_t size; + PyTypeObject *tp = Py_TYPE(dp); + _dbm_state *state = PyType_GetModuleState(tp); + if (state == NULL) { + return -1; + } if ((dp)->di_dbm == NULL) { - PyErr_SetString(DbmError, + PyErr_SetString(state->dbm_error, "DBM object has already been closed"); return -1; } @@ -266,22 +296,9 @@ dbm_contains(PyObject *self, PyObject *arg) return val.dptr != NULL; } -static PySequenceMethods dbm_as_sequence = { - 0, /* sq_length */ - 0, /* sq_concat */ - 0, /* sq_repeat */ - 0, /* sq_item */ - 0, /* sq_slice */ - 0, /* sq_ass_item */ - 0, /* sq_ass_slice */ - dbm_contains, /* sq_contains */ - 0, /* sq_inplace_concat */ - 0, /* sq_inplace_repeat */ -}; - /*[clinic input] _dbm.dbm.get - + cls: defining_class key: str(accept={str, robuffer}, zeroes=True) default: object = None / @@ -290,16 +307,18 @@ Return the value for key if present, otherwise default. [clinic start generated code]*/ static PyObject * -_dbm_dbm_get_impl(dbmobject *self, const char *key, +_dbm_dbm_get_impl(dbmobject *self, PyTypeObject *cls, const char *key, Py_ssize_clean_t key_length, PyObject *default_value) -/*[clinic end generated code: output=b44f95eba8203d93 input=b788eba0ffad2e91]*/ -/*[clinic end generated code: output=4f5c0e523eaf1251 input=9402c0af8582dc69]*/ +/*[clinic end generated code: output=34851b5dc1c664dc input=66b993b8349fa8c1]*/ { datum dbm_key, val; - + _dbm_state *state = PyType_GetModuleState(cls); + if (state == NULL) { + return NULL; + } dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; - check_dbmobject_open(self); + check_dbmobject_open(self, state->dbm_error); val = dbm_fetch(self->di_dbm, dbm_key); if (val.dptr != NULL) return PyBytes_FromStringAndSize(val.dptr, val.dsize); @@ -310,6 +329,7 @@ _dbm_dbm_get_impl(dbmobject *self, const char *key, /*[clinic input] _dbm.dbm.setdefault + cls: defining_class key: str(accept={str, robuffer}, zeroes=True) default: object(c_default="NULL") = b'' / @@ -320,17 +340,20 @@ If key is not in the database, it is inserted with default as the value. [clinic start generated code]*/ static PyObject * -_dbm_dbm_setdefault_impl(dbmobject *self, const char *key, +_dbm_dbm_setdefault_impl(dbmobject *self, PyTypeObject *cls, const char *key, Py_ssize_clean_t key_length, PyObject *default_value) -/*[clinic end generated code: output=52545886cf272161 input=bf40c48edaca01d6]*/ +/*[clinic end generated code: output=d5c68fe673886767 input=126a3ff15c5f8232]*/ { datum dbm_key, val; Py_ssize_t tmp_size; - + _dbm_state *state = PyType_GetModuleState(cls); + if (state == NULL) { + return NULL; + } dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; - check_dbmobject_open(self); + check_dbmobject_open(self, state->dbm_error); val = dbm_fetch(self->di_dbm, dbm_key); if (val.dptr != NULL) return PyBytes_FromStringAndSize(val.dptr, val.dsize); @@ -352,7 +375,7 @@ _dbm_dbm_setdefault_impl(dbmobject *self, const char *key, } if (dbm_store(self->di_dbm, dbm_key, val, DBM_INSERT) < 0) { dbm_clearerr(self->di_dbm); - PyErr_SetString(DbmError, "cannot add item to database"); + PyErr_SetString(state->dbm_error, "cannot add item to database"); Py_DECREF(default_value); return NULL; } @@ -373,7 +396,6 @@ dbm__exit__(PyObject *self, PyObject *args) return _PyObject_CallMethodIdNoArgs(self, &PyId_close); } - static PyMethodDef dbm_methods[] = { _DBM_DBM_CLOSE_METHODDEF _DBM_DBM_KEYS_METHODDEF @@ -381,38 +403,24 @@ static PyMethodDef dbm_methods[] = { _DBM_DBM_SETDEFAULT_METHODDEF {"__enter__", dbm__enter__, METH_NOARGS, NULL}, {"__exit__", dbm__exit__, METH_VARARGS, NULL}, - {NULL, NULL} /* sentinel */ + {NULL, NULL} /* sentinel */ +}; + +static PyType_Slot dbmtype_spec_slots[] = { + {Py_tp_dealloc, dbm_dealloc}, + {Py_tp_methods, dbm_methods}, + {Py_sq_contains, dbm_contains}, + {Py_mp_length, dbm_length}, + {Py_mp_subscript, dbm_subscript}, + {Py_mp_ass_subscript, dbm_ass_sub}, + {0, 0} }; -static PyTypeObject Dbmtype = { - PyVarObject_HEAD_INIT(NULL, 0) - "_dbm.dbm", - sizeof(dbmobject), - 0, - (destructor)dbm_dealloc, /*tp_dealloc*/ - 0, /*tp_vectorcall_offset*/ - 0, /*tp_getattr*/ - 0, /*tp_setattr*/ - 0, /*tp_as_async*/ - 0, /*tp_repr*/ - 0, /*tp_as_number*/ - &dbm_as_sequence, /*tp_as_sequence*/ - &dbm_as_mapping, /*tp_as_mapping*/ - 0, /*tp_hash*/ - 0, /*tp_call*/ - 0, /*tp_str*/ - 0, /*tp_getattro*/ - 0, /*tp_setattro*/ - 0, /*tp_as_buffer*/ - Py_TPFLAGS_DEFAULT, /*tp_flags*/ - 0, /*tp_doc*/ - 0, /*tp_traverse*/ - 0, /*tp_clear*/ - 0, /*tp_richcompare*/ - 0, /*tp_weaklistoffset*/ - 0, /*tp_iter*/ - 0, /*tp_iternext*/ - dbm_methods, /*tp_methods*/ +static PyType_Spec dbmtype_spec = { + .name = "_dbm.dbm", + .basicsize = sizeof(dbmobject), + .flags = Py_TPFLAGS_DEFAULT, + .slots = dbmtype_spec_slots, }; /* ----------------------------------------------------------------- */ @@ -443,7 +451,10 @@ dbmopen_impl(PyObject *module, PyObject *filename, const char *flags, /*[clinic end generated code: output=9527750f5df90764 input=376a9d903a50df59]*/ { int iflags; - + _dbm_state *state = get_dbm_state(module); + if (state == NULL) { + return NULL; + } if ( strcmp(flags, "r") == 0 ) iflags = O_RDONLY; else if ( strcmp(flags, "w") == 0 ) @@ -455,7 +466,7 @@ dbmopen_impl(PyObject *module, PyObject *filename, const char *flags, else if ( strcmp(flags, "n") == 0 ) iflags = O_RDWR|O_CREAT|O_TRUNC; else { - PyErr_SetString(DbmError, + PyErr_SetString(state->dbm_error, "arg 2 to open should be 'r', 'w', 'c', or 'n'"); return NULL; } @@ -470,7 +481,7 @@ dbmopen_impl(PyObject *module, PyObject *filename, const char *flags, PyErr_SetString(PyExc_ValueError, "embedded null character"); return NULL; } - PyObject *self = newdbmobject(name, iflags, mode); + PyObject *self = newdbmobject(state, name, iflags, mode); Py_DECREF(filenamebytes); return self; } @@ -480,42 +491,68 @@ static PyMethodDef dbmmodule_methods[] = { { 0, 0 }, }; +static int +_dbm_exec(PyObject *module) +{ + _dbm_state *state = get_dbm_state(module); + state->dbm_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, &dbmtype_spec, NULL); + if (state->dbm_type == NULL) { + return -1; + } + state->dbm_error = PyErr_NewException("_dbm.error", PyExc_OSError, NULL); + if (PyModule_AddStringConstant(module, "library", which_dbm) < 0) { + return -1; + } + Py_INCREF(state->dbm_error); + if (PyModule_AddObject(module, "error", state->dbm_error) < 0) { + Py_DECREF(state->dbm_error); + return -1; + } + return 0; +} + +static int +_dbm_module_traverse(PyObject *module, visitproc visit, void *arg) +{ + _dbm_state *state = get_dbm_state(module); + Py_VISIT(state->dbm_error); + Py_VISIT(state->dbm_type); + return 0; +} + +static int +_dbm_module_clear(PyObject *module) +{ + _dbm_state *state = get_dbm_state(module); + Py_CLEAR(state->dbm_error); + Py_CLEAR(state->dbm_type); + return 0; +} + +static void +_dbm_module_free(void *module) +{ + _dbm_module_clear((PyObject *)module); +} + +static PyModuleDef_Slot _dbmmodule_slots[] = { + {Py_mod_exec, _dbm_exec}, + {0, NULL} +}; static struct PyModuleDef _dbmmodule = { PyModuleDef_HEAD_INIT, - "_dbm", - NULL, - -1, - dbmmodule_methods, - NULL, - NULL, - NULL, - NULL + .m_name = "_dbm", + .m_size = sizeof(_dbm_state), + .m_methods = dbmmodule_methods, + .m_slots = _dbmmodule_slots, + .m_traverse = _dbm_module_traverse, + .m_clear = _dbm_module_clear, + .m_free = _dbm_module_free, }; PyMODINIT_FUNC -PyInit__dbm(void) { - PyObject *m, *d, *s; - - if (PyType_Ready(&Dbmtype) < 0) - return NULL; - m = PyModule_Create(&_dbmmodule); - if (m == NULL) - return NULL; - d = PyModule_GetDict(m); - if (DbmError == NULL) - DbmError = PyErr_NewException("_dbm.error", - PyExc_OSError, NULL); - s = PyUnicode_FromString(which_dbm); - if (s != NULL) { - PyDict_SetItemString(d, "library", s); - Py_DECREF(s); - } - if (DbmError != NULL) - PyDict_SetItemString(d, "error", DbmError); - if (PyErr_Occurred()) { - Py_DECREF(m); - m = NULL; - } - return m; +PyInit__dbm(void) +{ + return PyModuleDef_Init(&_dbmmodule); } diff --git a/Modules/clinic/_dbmmodule.c.h b/Modules/clinic/_dbmmodule.c.h index edf29be92af9bb..af288c2586a10b 100644 --- a/Modules/clinic/_dbmmodule.c.h +++ b/Modules/clinic/_dbmmodule.c.h @@ -27,15 +27,26 @@ PyDoc_STRVAR(_dbm_dbm_keys__doc__, "Return a list of all keys in the database."); #define _DBM_DBM_KEYS_METHODDEF \ - {"keys", (PyCFunction)_dbm_dbm_keys, METH_NOARGS, _dbm_dbm_keys__doc__}, + {"keys", (PyCFunction)(void(*)(void))_dbm_dbm_keys, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, _dbm_dbm_keys__doc__}, static PyObject * -_dbm_dbm_keys_impl(dbmobject *self); +_dbm_dbm_keys_impl(dbmobject *self, PyTypeObject *cls); static PyObject * -_dbm_dbm_keys(dbmobject *self, PyObject *Py_UNUSED(ignored)) +_dbm_dbm_keys(dbmobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { - return _dbm_dbm_keys_impl(self); + PyObject *return_value = NULL; + static const char * const _keywords[] = { NULL}; + static _PyArg_Parser _parser = {":keys", _keywords, 0}; + + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser + )) { + goto exit; + } + return_value = _dbm_dbm_keys_impl(self, cls); + +exit: + return return_value; } PyDoc_STRVAR(_dbm_dbm_get__doc__, @@ -45,25 +56,27 @@ PyDoc_STRVAR(_dbm_dbm_get__doc__, "Return the value for key if present, otherwise default."); #define _DBM_DBM_GET_METHODDEF \ - {"get", (PyCFunction)(void(*)(void))_dbm_dbm_get, METH_FASTCALL, _dbm_dbm_get__doc__}, + {"get", (PyCFunction)(void(*)(void))_dbm_dbm_get, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, _dbm_dbm_get__doc__}, static PyObject * -_dbm_dbm_get_impl(dbmobject *self, const char *key, +_dbm_dbm_get_impl(dbmobject *self, PyTypeObject *cls, const char *key, Py_ssize_clean_t key_length, PyObject *default_value); static PyObject * -_dbm_dbm_get(dbmobject *self, PyObject *const *args, Py_ssize_t nargs) +_dbm_dbm_get(dbmobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; + static const char * const _keywords[] = {"", "", NULL}; + static _PyArg_Parser _parser = {"s#|O:get", _keywords, 0}; const char *key; Py_ssize_clean_t key_length; PyObject *default_value = Py_None; - if (!_PyArg_ParseStack(args, nargs, "s#|O:get", + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, &key, &key_length, &default_value)) { goto exit; } - return_value = _dbm_dbm_get_impl(self, key, key_length, default_value); + return_value = _dbm_dbm_get_impl(self, cls, key, key_length, default_value); exit: return return_value; @@ -78,26 +91,28 @@ PyDoc_STRVAR(_dbm_dbm_setdefault__doc__, "If key is not in the database, it is inserted with default as the value."); #define _DBM_DBM_SETDEFAULT_METHODDEF \ - {"setdefault", (PyCFunction)(void(*)(void))_dbm_dbm_setdefault, METH_FASTCALL, _dbm_dbm_setdefault__doc__}, + {"setdefault", (PyCFunction)(void(*)(void))_dbm_dbm_setdefault, METH_METHOD|METH_FASTCALL|METH_KEYWORDS, _dbm_dbm_setdefault__doc__}, static PyObject * -_dbm_dbm_setdefault_impl(dbmobject *self, const char *key, +_dbm_dbm_setdefault_impl(dbmobject *self, PyTypeObject *cls, const char *key, Py_ssize_clean_t key_length, PyObject *default_value); static PyObject * -_dbm_dbm_setdefault(dbmobject *self, PyObject *const *args, Py_ssize_t nargs) +_dbm_dbm_setdefault(dbmobject *self, PyTypeObject *cls, PyObject *const *args, Py_ssize_t nargs, PyObject *kwnames) { PyObject *return_value = NULL; + static const char * const _keywords[] = {"", "", NULL}; + static _PyArg_Parser _parser = {"s#|O:setdefault", _keywords, 0}; const char *key; Py_ssize_clean_t key_length; PyObject *default_value = NULL; - if (!_PyArg_ParseStack(args, nargs, "s#|O:setdefault", + if (!_PyArg_ParseStackAndKeywords(args, nargs, kwnames, &_parser, &key, &key_length, &default_value)) { goto exit; } - return_value = _dbm_dbm_setdefault_impl(self, key, key_length, default_value); + return_value = _dbm_dbm_setdefault_impl(self, cls, key, key_length, default_value); exit: return return_value; @@ -172,4 +187,4 @@ dbmopen(PyObject *module, PyObject *const *args, Py_ssize_t nargs) exit: return return_value; } -/*[clinic end generated code: output=ba4ff07b8c8bbfe4 input=a9049054013a1b77]*/ +/*[clinic end generated code: output=6947b1115df66f7c input=a9049054013a1b77]*/ From ce98ee993663866aa317877f8a9515317e02d70c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sat, 13 Jun 2020 17:54:06 +0900 Subject: [PATCH 2/9] bpo-1635741: Apply PEP 7 --- Modules/_dbmmodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 11a6d86900d9f6..a0fbb85704cafe 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -495,7 +495,8 @@ static int _dbm_exec(PyObject *module) { _dbm_state *state = get_dbm_state(module); - state->dbm_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, &dbmtype_spec, NULL); + state->dbm_type = (PyTypeObject *)PyType_FromModuleAndSpec(module, + &dbmtype_spec, NULL); if (state->dbm_type == NULL) { return -1; } From f0371c6fcbbdc74c783a9f266f7a3bfef899c56b Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Sat, 13 Jun 2020 17:56:22 +0900 Subject: [PATCH 3/9] bpo-1635741: Apply PEP 7 --- Modules/_dbmmodule.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index a0fbb85704cafe..1329a383b39405 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -212,8 +212,9 @@ static PyObject * _dbm_dbm_close_impl(dbmobject *self) /*[clinic end generated code: output=c8dc5b6709600b86 input=046db72377d51be8]*/ { - if (self->di_dbm) + if (self->di_dbm) { dbm_close(self->di_dbm); + } self->di_dbm = NULL; Py_RETURN_NONE; } @@ -240,8 +241,9 @@ _dbm_dbm_keys_impl(dbmobject *self, PyTypeObject *cls) } check_dbmobject_open(self, state->dbm_error); v = PyList_New(0); - if (v == NULL) + if (v == NULL) { return NULL; + } for (key = dbm_firstkey(self->di_dbm); key.dptr; key = dbm_nextkey(self->di_dbm)) { item = PyBytes_FromStringAndSize(key.dptr, key.dsize); @@ -359,8 +361,9 @@ _dbm_dbm_setdefault_impl(dbmobject *self, PyTypeObject *cls, const char *key, return PyBytes_FromStringAndSize(val.dptr, val.dsize); if (default_value == NULL) { default_value = PyBytes_FromStringAndSize(NULL, 0); - if (default_value == NULL) + if (default_value == NULL) { return NULL; + } val.dptr = NULL; val.dsize = 0; } From 7ec3d5c79e117bd7760be3130570cfd8484d1f16 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Mon, 15 Jun 2020 23:34:04 +0900 Subject: [PATCH 4/9] bpo-1635741: Apply Victor's review --- Modules/_dbmmodule.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 1329a383b39405..858e52f8ff3326 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -84,11 +84,11 @@ newdbmobject(_dbm_state *state, const char *file, int flags, int mode) static void dbm_dealloc(dbmobject *dp) { - if ( dp->di_dbm ) + if (dp->di_dbm) { dbm_close(dp->di_dbm); + } PyTypeObject *tp = Py_TYPE(dp); - freefunc free_func = PyType_GetSlot(tp, Py_tp_free); - free_func(dp); + tp->tp_free(dp); Py_DECREF(tp); } @@ -97,6 +97,7 @@ dbm_length(dbmobject *dp) { PyTypeObject *tp = Py_TYPE(dp); _dbm_state *state = PyType_GetModuleState(tp); + assert(state != NULL); if (state == NULL) { return -1; } @@ -419,9 +420,14 @@ static PyType_Slot dbmtype_spec_slots[] = { {0, 0} }; + static PyType_Spec dbmtype_spec = { .name = "_dbm.dbm", .basicsize = sizeof(dbmobject), + /* + Calling PyType_GetModuleState() is safe + because Py_TPFLAGS_BASETYPE flag is not used. + */ .flags = Py_TPFLAGS_DEFAULT, .slots = dbmtype_spec_slots, }; From 4a69834d9e391b55f711323b9154e8c24948cf28 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 16 Jun 2020 00:11:40 +0900 Subject: [PATCH 5/9] bpo-1635741: Apply Victor's review --- Modules/_dbmmodule.c | 54 +++++++++++++++++++------------------------- 1 file changed, 23 insertions(+), 31 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 858e52f8ff3326..a66eb979171119 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -98,9 +98,6 @@ dbm_length(dbmobject *dp) PyTypeObject *tp = Py_TYPE(dp); _dbm_state *state = PyType_GetModuleState(tp); assert(state != NULL); - if (state == NULL) { - return -1; - } if (dp->di_dbm == NULL) { PyErr_SetString(state->dbm_error, "DBM object has already been closed"); return -1; @@ -125,11 +122,10 @@ dbm_subscript(dbmobject *dp, PyObject *key) Py_ssize_t tmp_size; PyTypeObject *tp = Py_TYPE(dp); _dbm_state *state = PyType_GetModuleState(tp); - if (state == NULL) { + assert(state != NULL); + if (!PyArg_Parse(key, "s#", &krec.dptr, &tmp_size)) { return NULL; } - if (!PyArg_Parse(key, "s#", &krec.dptr, &tmp_size) ) - return NULL; krec.dsize = tmp_size; check_dbmobject_open(dp, state->dbm_error); @@ -159,9 +155,7 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w) } PyTypeObject *tp = Py_TYPE(dp); _dbm_state *state = PyType_GetModuleState(tp); - if (state == NULL) { - return -1; - } + assert(state != NULL); krec.dsize = tmp_size; if (dp->di_dbm == NULL) { PyErr_SetString(state->dbm_error, "DBM object has already been closed"); @@ -237,9 +231,7 @@ _dbm_dbm_keys_impl(dbmobject *self, PyTypeObject *cls) int err; _dbm_state *state = PyType_GetModuleState(cls); - if (state == NULL) { - return NULL; - } + assert(state != NULL); check_dbmobject_open(self, state->dbm_error); v = PyList_New(0); if (v == NULL) { @@ -271,9 +263,7 @@ dbm_contains(PyObject *self, PyObject *arg) PyTypeObject *tp = Py_TYPE(dp); _dbm_state *state = PyType_GetModuleState(tp); - if (state == NULL) { - return -1; - } + assert(state != NULL); if ((dp)->di_dbm == NULL) { PyErr_SetString(state->dbm_error, "DBM object has already been closed"); @@ -316,15 +306,14 @@ _dbm_dbm_get_impl(dbmobject *self, PyTypeObject *cls, const char *key, { datum dbm_key, val; _dbm_state *state = PyType_GetModuleState(cls); - if (state == NULL) { - return NULL; - } + assert(state != NULL); dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; check_dbmobject_open(self, state->dbm_error); val = dbm_fetch(self->di_dbm, dbm_key); - if (val.dptr != NULL) + if (val.dptr != NULL) { return PyBytes_FromStringAndSize(val.dptr, val.dsize); + } Py_INCREF(default_value); return default_value; @@ -351,15 +340,14 @@ _dbm_dbm_setdefault_impl(dbmobject *self, PyTypeObject *cls, const char *key, datum dbm_key, val; Py_ssize_t tmp_size; _dbm_state *state = PyType_GetModuleState(cls); - if (state == NULL) { - return NULL; - } + assert(state != NULL); dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; check_dbmobject_open(self, state->dbm_error); val = dbm_fetch(self->di_dbm, dbm_key); - if (val.dptr != NULL) + if (val.dptr != NULL) { return PyBytes_FromStringAndSize(val.dptr, val.dsize); + } if (default_value == NULL) { default_value = PyBytes_FromStringAndSize(NULL, 0); if (default_value == NULL) { @@ -461,19 +449,23 @@ dbmopen_impl(PyObject *module, PyObject *filename, const char *flags, { int iflags; _dbm_state *state = get_dbm_state(module); - if (state == NULL) { - return NULL; - } - if ( strcmp(flags, "r") == 0 ) + assert(state != NULL); + if (strcmp(flags, "r") == 0) { iflags = O_RDONLY; - else if ( strcmp(flags, "w") == 0 ) + } + else if (strcmp(flags, "w") == 0) { iflags = O_RDWR; - else if ( strcmp(flags, "rw") == 0 ) /* B/W compat */ + } + else if (strcmp(flags, "rw") == 0) { + /* B/W compat */ iflags = O_RDWR|O_CREAT; - else if ( strcmp(flags, "c") == 0 ) + } + else if (strcmp(flags, "c") == 0) { iflags = O_RDWR|O_CREAT; - else if ( strcmp(flags, "n") == 0 ) + } + else if (strcmp(flags, "n") == 0) { iflags = O_RDWR|O_CREAT|O_TRUNC; + } else { PyErr_SetString(state->dbm_error, "arg 2 to open should be 'r', 'w', 'c', or 'n'"); From 87a6ef12b73a0ed3a63705125db33b52eaf79e9e Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 16 Jun 2020 00:20:39 +0900 Subject: [PATCH 6/9] bpo-1635741: reformat macro --- Modules/_dbmmodule.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index a66eb979171119..847c58dccb50df 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -56,9 +56,11 @@ typedef struct { #include "clinic/_dbmmodule.c.h" -#define check_dbmobject_open(v, err) if ((v)->di_dbm == NULL) \ - { PyErr_SetString(err, "DBM object has already been closed"); \ - return NULL; } +#define check_dbmobject_open(v, err) \ +if ((v)->di_dbm == NULL) { \ + PyErr_SetString(err, "DBM object has already been closed"); \ + return NULL; \ +} static PyObject * newdbmobject(_dbm_state *state, const char *file, int flags, int mode) From b325856a59a7b0f7b65a3b3c99475403626b94a8 Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 16 Jun 2020 00:13:25 +0900 Subject: [PATCH 7/9] Update Modules/_dbmmodule.c Co-authored-by: Victor Stinner --- Modules/_dbmmodule.c | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 847c58dccb50df..bcb0ad31b336ef 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -414,10 +414,9 @@ static PyType_Slot dbmtype_spec_slots[] = { static PyType_Spec dbmtype_spec = { .name = "_dbm.dbm", .basicsize = sizeof(dbmobject), - /* - Calling PyType_GetModuleState() is safe - because Py_TPFLAGS_BASETYPE flag is not used. - */ + // Calling PyType_GetModuleState() on a subclass is not safe. + // dbmtype_spec does not have Py_TPFLAGS_BASETYPE flag which prevents to create a subclass. + // So calling PyType_GetModuleState() in this file is always safe. .flags = Py_TPFLAGS_DEFAULT, .slots = dbmtype_spec_slots, }; From 46d7c6903c15bc83807acec7c3f06acff3114d3a Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 16 Jun 2020 00:24:14 +0900 Subject: [PATCH 8/9] bpo-1635741: Apply code review --- Modules/_dbmmodule.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index bcb0ad31b336ef..93e98800394f6b 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -97,8 +97,7 @@ dbm_dealloc(dbmobject *dp) static Py_ssize_t dbm_length(dbmobject *dp) { - PyTypeObject *tp = Py_TYPE(dp); - _dbm_state *state = PyType_GetModuleState(tp); + _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp)); assert(state != NULL); if (dp->di_dbm == NULL) { PyErr_SetString(state->dbm_error, "DBM object has already been closed"); @@ -122,8 +121,7 @@ dbm_subscript(dbmobject *dp, PyObject *key) { datum drec, krec; Py_ssize_t tmp_size; - PyTypeObject *tp = Py_TYPE(dp); - _dbm_state *state = PyType_GetModuleState(tp); + _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp)); assert(state != NULL); if (!PyArg_Parse(key, "s#", &krec.dptr, &tmp_size)) { return NULL; @@ -155,8 +153,7 @@ dbm_ass_sub(dbmobject *dp, PyObject *v, PyObject *w) "dbm mappings have bytes or string keys only"); return -1; } - PyTypeObject *tp = Py_TYPE(dp); - _dbm_state *state = PyType_GetModuleState(tp); + _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp)); assert(state != NULL); krec.dsize = tmp_size; if (dp->di_dbm == NULL) { @@ -263,8 +260,7 @@ dbm_contains(PyObject *self, PyObject *arg) datum key, val; Py_ssize_t size; - PyTypeObject *tp = Py_TYPE(dp); - _dbm_state *state = PyType_GetModuleState(tp); + _dbm_state *state = PyType_GetModuleState(Py_TYPE(dp)); assert(state != NULL); if ((dp)->di_dbm == NULL) { PyErr_SetString(state->dbm_error, @@ -307,7 +303,7 @@ _dbm_dbm_get_impl(dbmobject *self, PyTypeObject *cls, const char *key, /*[clinic end generated code: output=34851b5dc1c664dc input=66b993b8349fa8c1]*/ { datum dbm_key, val; - _dbm_state *state = PyType_GetModuleState(cls); + _dbm_state *state = PyType_GetModuleState(cls); assert(state != NULL); dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; @@ -341,7 +337,7 @@ _dbm_dbm_setdefault_impl(dbmobject *self, PyTypeObject *cls, const char *key, { datum dbm_key, val; Py_ssize_t tmp_size; - _dbm_state *state = PyType_GetModuleState(cls); + _dbm_state *state = PyType_GetModuleState(cls); assert(state != NULL); dbm_key.dptr = (char *)key; dbm_key.dsize = key_length; From ae11a49dee419d5f97e86d9437debd2b7fea9a7c Mon Sep 17 00:00:00 2001 From: Dong-hee Na Date: Tue, 16 Jun 2020 00:52:34 +0900 Subject: [PATCH 9/9] bpo-1635741: Update --- Modules/_dbmmodule.c | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Modules/_dbmmodule.c b/Modules/_dbmmodule.c index 93e98800394f6b..97772a04d08fe0 100644 --- a/Modules/_dbmmodule.c +++ b/Modules/_dbmmodule.c @@ -56,11 +56,11 @@ typedef struct { #include "clinic/_dbmmodule.c.h" -#define check_dbmobject_open(v, err) \ -if ((v)->di_dbm == NULL) { \ - PyErr_SetString(err, "DBM object has already been closed"); \ - return NULL; \ -} +#define check_dbmobject_open(v, err) \ + if ((v)->di_dbm == NULL) { \ + PyErr_SetString(err, "DBM object has already been closed"); \ + return NULL; \ + } static PyObject * newdbmobject(_dbm_state *state, const char *file, int flags, int mode) @@ -411,7 +411,8 @@ static PyType_Spec dbmtype_spec = { .name = "_dbm.dbm", .basicsize = sizeof(dbmobject), // Calling PyType_GetModuleState() on a subclass is not safe. - // dbmtype_spec does not have Py_TPFLAGS_BASETYPE flag which prevents to create a subclass. + // dbmtype_spec does not have Py_TPFLAGS_BASETYPE flag + // which prevents to create a subclass. // So calling PyType_GetModuleState() in this file is always safe. .flags = Py_TPFLAGS_DEFAULT, .slots = dbmtype_spec_slots, @@ -454,7 +455,7 @@ dbmopen_impl(PyObject *module, PyObject *filename, const char *flags, iflags = O_RDWR; } else if (strcmp(flags, "rw") == 0) { - /* B/W compat */ + /* Backward compatibility */ iflags = O_RDWR|O_CREAT; } else if (strcmp(flags, "c") == 0) { @@ -499,12 +500,13 @@ _dbm_exec(PyObject *module) return -1; } state->dbm_error = PyErr_NewException("_dbm.error", PyExc_OSError, NULL); + if (state->dbm_error == NULL) { + return -1; + } if (PyModule_AddStringConstant(module, "library", which_dbm) < 0) { return -1; } - Py_INCREF(state->dbm_error); - if (PyModule_AddObject(module, "error", state->dbm_error) < 0) { - Py_DECREF(state->dbm_error); + if (PyModule_AddType(module, (PyTypeObject *)state->dbm_error) < 0) { return -1; } return 0;