[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