Skip to content

bpo-14935: Remove static state from the _csv module #23224

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Dec 15, 2020
Merged

bpo-14935: Remove static state from the _csv module #23224

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
a2a0e6f
Remove static state from the _csv module
encukou May 12, 2020
09802b2
Use PyModule_AddObjectRef and PyTuple_Pack
encukou Nov 10, 2020
1665ac7
Rename m_state to module_state
encukou Nov 13, 2020
23f844a
Use Py_CLEAR rather than Py_XDECREF
encukou Nov 13, 2020
8d1f95d
Use `module` rather than `m`
encukou Nov 13, 2020
2c60895
Factor out calling _PyType_GetModuleByDef & PyModule_GetState
encukou Nov 13, 2020
e69d0a1
Set freed pointers to NULL
encukou Nov 13, 2020
fb93e7f
Only call get_csv_state up to once per function
encukou Nov 13, 2020
b62987b
Pass module_state directly to _call_dialect
encukou Nov 13, 2020
f12a768
Add NULL check for _csv_state_from_type
encukou Dec 8, 2020
f302029
Use Py_NewRef where appropriate
encukou Dec 8, 2020
b8c37bf
Use designated initializers for spec structs
encukou Dec 8, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Rename m_state to module_state
  • Loading branch information
@encukou
encukou committed Nov 13, 2020
commit 1665ac7f067d4a0d05cd57a6e0a5754adf4d255a
110 changes: 56 additions & 54 deletions Modules/_csv.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,24 @@ get_csv_state(PyObject *module)
static int
_csv_clear(PyObject *m)
{
_csvstate *m_state = PyModule_GetState(m);
Py_CLEAR(m_state->error_obj);
Py_CLEAR(m_state->dialects);
Py_CLEAR(m_state->dialect_type);
Py_CLEAR(m_state->reader_type);
Py_CLEAR(m_state->writer_type);
_csvstate *module_state = PyModule_GetState(m);
Py_CLEAR(module_state->error_obj);
Py_CLEAR(module_state->dialects);
Py_CLEAR(module_state->dialect_type);
Py_CLEAR(module_state->reader_type);
Py_CLEAR(module_state->writer_type);
return 0;
}

static int
_csv_traverse(PyObject *m, visitproc visit, void *arg)
{
_csvstate *m_state = PyModule_GetState(m);
Py_VISIT(m_state->error_obj);
Py_VISIT(m_state->dialects);
Py_VISIT(m_state->dialect_type);
Py_VISIT(m_state->reader_type);
Py_VISIT(m_state->writer_type);
_csvstate *module_state = PyModule_GetState(m);
Py_VISIT(module_state->error_obj);
Py_VISIT(module_state->dialects);
Py_VISIT(module_state->dialect_type);
Py_VISIT(module_state->reader_type);
Py_VISIT(module_state->writer_type);
return 0;
}

Expand Down Expand Up @@ -137,14 +137,14 @@ typedef struct {
*/

static PyObject *
get_dialect_from_registry(PyObject *name_obj, _csvstate *m_state)
get_dialect_from_registry(PyObject *name_obj, _csvstate *module_state)
{
PyObject *dialect_obj;

dialect_obj = PyDict_GetItemWithError(m_state->dialects, name_obj);
dialect_obj = PyDict_GetItemWithError(module_state->dialects, name_obj);
if (dialect_obj == NULL) {
if (!PyErr_Occurred())
PyErr_Format(m_state->error_obj, "unknown dialect");
PyErr_Format(module_state->error_obj, "unknown dialect");
}
else
Py_INCREF(dialect_obj);
Expand Down Expand Up @@ -372,23 +372,23 @@ dialect_new(PyTypeObject *type, PyObject *args, PyObject *kwargs)
if (m == NULL) {
return NULL;
}
_csvstate *m_state = PyModule_GetState(m);
if (m_state == NULL) {
_csvstate *module_state = PyModule_GetState(m);
if (module_state == NULL) {
return PyErr_Format(PyExc_SystemError,
"dialect_new: No _csv module state found");
}


if (dialect != NULL) {
if (PyUnicode_Check(dialect)) {
dialect = get_dialect_from_registry(dialect, m_state);
dialect = get_dialect_from_registry(dialect, module_state);
if (dialect == NULL)
return NULL;
}
else
Py_INCREF(dialect);
/* Can we reuse this instance? */
if (PyObject_TypeCheck(dialect, m_state->dialect_type) &&
if (PyObject_TypeCheck(dialect, module_state->dialect_type) &&
delimiter == NULL &&
doublequote == NULL &&
escapechar == NULL &&
Expand Down Expand Up @@ -529,12 +529,12 @@ PyType_Spec Dialect_Type_spec = {
static PyObject *
_call_dialect(PyObject *m, PyObject *dialect_inst, PyObject *kwargs)
{
_csvstate *m_state = PyModule_GetState(m);
if (m_state == NULL) {
_csvstate *module_state = PyModule_GetState(m);
if (module_state == NULL) {
return NULL;
}

PyObject *type = (PyObject *)m_state->dialect_type;
PyObject *type = (PyObject *)module_state->dialect_type;
if (dialect_inst) {
return PyObject_VectorcallDict(type, &dialect_inst, 1, kwargs);
}
Expand Down Expand Up @@ -592,11 +592,12 @@ parse_grow_buff(ReaderObj *self)
}

static int
parse_add_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
parse_add_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c)
{
if (self->field_len >= m_state->field_limit) {
PyErr_Format(m_state->error_obj, "field larger than field limit (%ld)",
m_state->field_limit);
if (self->field_len >= module_state->field_limit) {
PyErr_Format(module_state->error_obj,
"field larger than field limit (%ld)",
module_state->field_limit);
return -1;
}
if (self->field_len == self->field_size && !parse_grow_buff(self))
Expand All @@ -606,7 +607,7 @@ parse_add_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
}

static int
parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
parse_process_char(ReaderObj *self, _csvstate *module_state, Py_UCS4 c)
{
DialectObj *dialect = self->dialect;

Expand Down Expand Up @@ -652,22 +653,22 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
/* begin new unquoted field */
if (dialect->quoting == QUOTE_NONNUMERIC)
self->numeric_field = 1;
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = IN_FIELD;
}
break;

case ESCAPED_CHAR:
if (c == '\n' || c=='\r') {
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = AFTER_ESCAPED_CRNL;
break;
}
if (c == '\0')
c = '\n';
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = IN_FIELD;
break;
Expand Down Expand Up @@ -697,7 +698,7 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
}
else {
/* normal character - save in field */
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
}
break;
Expand All @@ -723,15 +724,15 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
}
else {
/* normal character - save in field */
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
}
break;

case ESCAPE_IN_QUOTED_FIELD:
if (c == '\0')
c = '\n';
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = IN_QUOTED_FIELD;
break;
Expand All @@ -741,7 +742,7 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
if (dialect->quoting != QUOTE_NONE &&
c == dialect->quotechar) {
/* save "" as " */
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = IN_QUOTED_FIELD;
}
Expand All @@ -758,13 +759,13 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
self->state = (c == '\0' ? START_RECORD : EAT_CRNL);
}
else if (!dialect->strict) {
if (parse_add_char(self, m_state, c) < 0)
if (parse_add_char(self, module_state, c) < 0)
return -1;
self->state = IN_FIELD;
}
else {
/* illegal */
PyErr_Format(m_state->error_obj, "'%c' expected after '%c'",
PyErr_Format(module_state->error_obj, "'%c' expected after '%c'",
dialect->delimiter,
dialect->quotechar);
return -1;
Expand All @@ -777,7 +778,7 @@ parse_process_char(ReaderObj *self, _csvstate *m_state, Py_UCS4 c)
else if (c == '\0')
self->state = START_RECORD;
else {
PyErr_Format(m_state->error_obj,
PyErr_Format(module_state->error_obj,
"new-line character seen in unquoted field - do you need to open the file in universal-newline mode?");
return -1;
}
Expand Down Expand Up @@ -813,8 +814,8 @@ Reader_iternext(ReaderObj *self)
if (m == NULL) {
return NULL;
}
_csvstate *m_state = PyModule_GetState(m);
if (m_state == NULL) {
_csvstate *module_state = PyModule_GetState(m);
if (module_state == NULL) {
return PyErr_Format(PyExc_SystemError,
"dialect_new: No _csv module state found");
}
Expand All @@ -828,15 +829,15 @@ Reader_iternext(ReaderObj *self)
if (!PyErr_Occurred() && (self->field_len != 0 ||
self->state == IN_QUOTED_FIELD)) {
if (self->dialect->strict)
PyErr_SetString(m_state->error_obj,
PyErr_SetString(module_state->error_obj,
"unexpected end of data");
else if (parse_save_field(self) >= 0)
break;
}
return NULL;
}
if (!PyUnicode_Check(lineobj)) {
PyErr_Format(m_state->error_obj,
PyErr_Format(module_state->error_obj,
"iterator should return strings, "
"not %.200s "
"(the file should be opened in text mode)",
Expand All @@ -858,18 +859,18 @@ Reader_iternext(ReaderObj *self)
c = PyUnicode_READ(kind, data, pos);
if (c == '\0') {
Py_DECREF(lineobj);
PyErr_Format(m_state->error_obj,
PyErr_Format(module_state->error_obj,
"line contains NUL");
goto err;
}
if (parse_process_char(self, m_state, c) < 0) {
if (parse_process_char(self, module_state, c) < 0) {
Py_DECREF(lineobj);
goto err;
}
pos++;
}
Py_DECREF(lineobj);
if (parse_process_char(self, m_state, 0) < 0)
if (parse_process_char(self, module_state, 0) < 0)
goto err;
} while (self->state != START_RECORD);

Expand Down Expand Up @@ -1651,22 +1652,22 @@ static int
csv_exec(PyObject *module) {
const StyleDesc *style;
PyObject *temp;
_csvstate *m_state = get_csv_state(module);
_csvstate *module_state = get_csv_state(module);

temp = PyType_FromModuleAndSpec(module, &Dialect_Type_spec, NULL);
m_state->dialect_type = (PyTypeObject *)temp;
module_state->dialect_type = (PyTypeObject *)temp;
if (PyModule_AddObjectRef(module, "Dialect", temp) < 0) {
return -1;
}

temp = PyType_FromModuleAndSpec(module, &Reader_Type_spec, NULL);
m_state->reader_type = (PyTypeObject *)temp;
module_state->reader_type = (PyTypeObject *)temp;
if (PyModule_AddObjectRef(module, "Reader", temp) < 0) {
return -1;
}

temp = PyType_FromModuleAndSpec(module, &Writer_Type_spec, NULL);
m_state->writer_type = (PyTypeObject *)temp;
module_state->writer_type = (PyTypeObject *)temp;
if (PyModule_AddObjectRef(module, "Writer", temp) < 0) {
return -1;
}
Expand All @@ -1678,11 +1679,11 @@ csv_exec(PyObject *module) {
}

/* Set the field limit */
m_state->field_limit = 128 * 1024;
module_state->field_limit = 128 * 1024;

/* Add _dialects dictionary */
m_state->dialects = PyDict_New();
if (PyModule_AddObjectRef(module, "_dialects", m_state->dialects) < 0) {
module_state->dialects = PyDict_New();
if (PyModule_AddObjectRef(module, "_dialects", module_state->dialects) < 0) {
return -1;
}

Expand All @@ -1698,12 +1699,13 @@ csv_exec(PyObject *module) {
if (bases == NULL) {
return -1;
}
m_state->error_obj = PyType_FromModuleAndSpec(module, &error_spec, bases);
module_state->error_obj = PyType_FromModuleAndSpec(module, &error_spec,
bases);
Py_DECREF(bases);
if (m_state->error_obj == NULL) {
if (module_state->error_obj == NULL) {
return -1;
}
if (PyModule_AddType(module, (PyTypeObject *)m_state->error_obj) != 0) {
if (PyModule_AddType(module, (PyTypeObject *)module_state->error_obj) != 0) {
return -1;
}

Expand Down