[Csv] code review
Neal Norwitz
neal at metaslash.com
Sun Mar 23 03:35:39 CET 2003
Here are notes about the version of CSV checked in to the Python 2.3 tree.
* remove TODO comment at top of file--it's empty
* is CSV going to be maintained outside the python tree?
If not, remove the 2.2 compatibility macros for:
PyDoc_STR, PyDoc_STRVAR, PyMODINIT_FUNC, etc.
* inline the following functions since they are used only in one place
get_string, set_string, get_nullchar_as_None, set_nullchar_as_None,
join_reset (maybe)
* rather than use PyErr_BadArgument, should you use assert?
(first example, Dialect_set_quoting, line 218)
* is it necessary to have Dialect_methods, can you use 0 for tp_methods?
* remove commented out code (PyMem_DEL) on line 261
Have you used valgrind on the test to find memory overwrites/leaks?
* PyString_AsString()[0] on line 331 could return NULL in which case
you are dereferencing a NULL pointer
* note sure why there are casts on 0 pointers
lines 383-393, 733-743, 1144-1154, 1164-1165
* Reader_getiter() can be removed and use PyObject_SelfIter()
* I think you need PyErr_NoMemory() before returning on line 768, 1178
* is PyString_AsString(self->dialect->lineterminator) on line 994
guaranteed not to return NULL? If not, it could crash by
passing to memmove.
* PyString_AsString() can return NULL on line 1048 and 1063,
the result is passed to join_append()
* iteratable should be iterable? (line 1088)
* why doesn't csv_writerows() have a docstring? csv_writerow does
* any PyUnicode_* methods should be protected with #ifdef Py_USING_UNICODE
* csv_unregister_dialect, csv_get_dialect could use METH_O
so you don't need to use PyArg_ParseTuple
* in init_csv, recommend using
PyModule_AddIntConstant and PyModule_AddStringConstant
where appropriate
Neal
More information about the Csv
mailing list