[Csv] csv to-do

Skip Montanaro skip at pobox.com
Sat Apr 12 01:15:43 CEST 2003


To do list:

    * catch docs up to the current code (Skip)

    * make csv module gc-aware (Jeremy - said he'd do this)

    * Neal's code review feedback:

        - remove TODO comment at top of file--it's empty

          (done I think)

        - 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.

          (rationale for leaving them in explained already)

        - 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)

          (Dave or Andrew)

        - rather than use PyErr_BadArgument, should you use assert?
                (first example, Dialect_set_quoting, line 218)

          (Dave or Andrew)

        - is it necessary to have Dialect_methods, can you use 0 for
          tp_methods? 

        - remove commented out code (PyMem_DEL) on line 261

          (done)

        - Have you used valgrind on the test to find memory
          overwrites/leaks?

          (Dave or Andrew)

        - PyString_AsString()[0] on line 331 could return NULL in which case
          you are dereferencing a NULL pointer

          (Skip)

        - note sure why there are casts on 0 pointers lines 383-393,
          733-743, 1144-1154, 1164-1165

          (I think this refers to the various static PyTypeObjects.  I
          believe the convention normally is to apply the casts.)
        
        - Reader_getiter() can be removed and use PyObject_SelfIter()

          (Dave or Andrew)

        - I think you need PyErr_NoMemory() before returning on line 768, 1178

          (Dave or Andrew)

        - is PyString_AsString(self->dialect->lineterminator) on line 994
          guaranteed not to return NULL?  If not, it could crash by passing
          to memmove.

          (Dave or Andrew)

        - PyString_AsString() can return NULL on line 1048 and 1063, the
          result is passed to join_append()

          (Dave or Andrew)

        - iteratable should be iterable?  (line 1088)

          (done)

        - why doesn't csv_writerows() have a docstring?  csv_writerow does

          (done - still to checkin)

        - any PyUnicode_* methods should be protected with #ifdef
          Py_USING_UNICODE 

          (Skip)

        - csv_unregister_dialect, csv_get_dialect could use METH_O so you
          don't need to use PyArg_ParseTuple 

          (Skip)

        - in init_csv, recommend using PyModule_AddIntConstant and
          PyModule_AddStringConstant where appropriate

          (Skip)


More information about the Csv mailing list