[Python-checkins] python/dist/src/Python codecs.c,2.14,2.15 exceptions.c,1.34,1.35

[Neal Norwitz]

Some code review comments below. -- Neal

doerwalter@users.sourceforge.net wrote:

Python/codecs.c:

> + static void wrong_exception_type(PyObject *exc)
> + {
> +     PyObject *type = PyObject_GetAttrString(exc, "__class__");
> +     if (type != NULL) {
> +       PyObject *name = PyObject_GetAttrString(type, "__name__");
> +       Py_DECREF(type);
> +       if (name != NULL) {
> +           PyObject *string = PyObject_Str(name);
> +           Py_DECREF(name);
> +           PyErr_Format(PyExc_TypeError, "don't know how to handle %.400s in error callback",
> +               PyString_AS_STRING(string));
> +           Py_DECREF(string);
> +       }
> +     }
> + }

PyObject_Str() can return NULL.  Perhaps string should be checked,
although it should never be NULL.

> + PyObject *PyCodec_ReplaceErrors(PyObject *exc)
> + {
	...
> +       for (p = PyUnicode_AS_UNICODE(res), i = start;
> +           i<end; ++p, ++i)
> +           *p = '?';

The for loop can be rewritten as:
	  memset(PyUnicode_AS_UNICODE(res) + start, '?', end - start);

> +       for (p = PyUnicode_AS_UNICODE(res), i = start;
> +           i<end; ++p, ++i)
> +           *p = Py_UNICODE_REPLACEMENT_CHARACTER;

Same here.

> Index: exceptions.c
> ***************
> *** 841,844 ****
> --- 845,1432 ----
> 
> + int get_int(PyObject *exc, const char *name, int *value)
> + {
> +     PyObject *attr = PyObject_GetAttrString(exc, (char *)name);
> +
> +     if (!attr)
> +       return -1;
> +     if (!PyInt_Check(attr)) {
> +       PyErr_Format(PyExc_TypeError, "%s attribute must be int", name);

Should limit %s to %.200s or some other value.

> + PyObject *get_string(PyObject *exc, const char *name)
> + {
> +     PyObject *attr = PyObject_GetAttrString(exc, (char *)name);
> +
> +     if (!attr)
> +       return NULL;
> +     if (!PyString_Check(attr)) {
> +       PyErr_Format(PyExc_TypeError, "%s attribute must be str", name);

Same.

> + PyObject *get_unicode(PyObject *exc, const char *name)
> + {
> +     PyObject *attr = PyObject_GetAttrString(exc, (char *)name);
> +
> +     if (!attr)
> +       return NULL;
> +     if (!PyUnicode_Check(attr)) {
> +       PyErr_Format(PyExc_TypeError, "%s attribute must be unicode", name);

Same.

> + PyObject * PyUnicodeEncodeError_GetEncoding(PyObject *exc)
> + {
> +     return get_string(exc, "encoding");
> + }
> +
> + PyObject * PyUnicodeDecodeError_GetEncoding(PyObject *exc)
> + {
> +     return get_string(exc, "encoding");
> + }
> +
> + PyObject * PyUnicodeTranslateError_GetEncoding(PyObject *exc)
> + {
> +     return get_string(exc, "encoding");
> + }

Are these supposed to be the same?  Can there be a single API?

> + PyObject *PyUnicodeEncodeError_GetObject(PyObject *exc)
> + {
> +     return get_unicode(exc, "object");
> + }
> +
> + PyObject *PyUnicodeDecodeError_GetObject(PyObject *exc)
> + {
> +     return get_string(exc, "object");
> + }
> +
> + PyObject *PyUnicodeTranslateError_GetObject(PyObject *exc)
> + {
> +     return get_unicode(exc, "object");
> + }

Are the Encode/Translate supposed to be unicode, 
while Decode returns a string?  Are all 3 necessary?

> + int PyUnicodeEncodeError_SetStart(PyObject *exc, int start)
> + {
> +     return set_int(exc, "start", start);
> + }
> +
> +
> + int PyUnicodeDecodeError_SetStart(PyObject *exc, int start)
> + {
> +     return set_int(exc, "start", start);
> + }
> +
> +
> + int PyUnicodeTranslateError_SetStart(PyObject *exc, int start)
> + {
> +     return set_int(exc, "start", start);
> + }

Same comment.

> + int PyUnicodeEncodeError_SetEnd(PyObject *exc, int end)
> + {
> +     return set_int(exc, "end", end);
> + }
> +
> +
> + int PyUnicodeDecodeError_SetEnd(PyObject *exc, int end)
> + {
> +     return set_int(exc, "end", end);
> + }
> +
> +
> + int PyUnicodeTranslateError_SetEnd(PyObject *exc, int end)
> + {
> +     return set_int(exc, "end", end);
> + }
> +

Same comment.

> + PyObject *PyUnicodeEncodeError_GetReason(PyObject *exc)
> + {
> +     return get_string(exc, "reason");
> + }
> +
> +
> + PyObject *PyUnicodeDecodeError_GetReason(PyObject *exc)
> + {
> +     return get_string(exc, "reason");
> + }
> +
> +
> + PyObject *PyUnicodeTranslateError_GetReason(PyObject *exc)
> + {
> +     return get_string(exc, "reason");
> + }

Same comment.

> + int PyUnicodeEncodeError_SetReason(PyObject *exc, const char *reason)
> + {
> +     return set_string(exc, "reason", reason);
> + }
> +
> +
> + int PyUnicodeDecodeError_SetReason(PyObject *exc, const char *reason)
> + {
> +     return set_string(exc, "reason", reason);
> + }
> +
> +
> + int PyUnicodeTranslateError_SetReason(PyObject *exc, const char *reason)
> + {
> +     return set_string(exc, "reason", reason);
> + }

Same comment.

> + UnicodeTranslateError__init__(PyObject *self, PyObject *args)

Seems to be the same as UnicodeError__init__().