Missing NULL return checks?
PyArray_DescrFromType can return NULL static PyArray_Descr * PyArray_DescrFromType(int type) { PyArray_Descr *ret = NULL; if (type < PyArray_NTYPES) { ret = _builtin_descrs[type]; } else if (type == PyArray_NOTYPE) { /* * This needs to not raise an error so * that PyArray_DescrFromType(PyArray_NOTYPE) * works for backwards-compatible C-API */ return NULL; } else if ((type == PyArray_CHAR) || (type == PyArray_CHARLTR)) { ret = PyArray_DescrNew(_builtin_descrs[PyArray_STRING]); if (ret == NULL) { return NULL; } ret->elsize = 1; ret->type = PyArray_CHARLTR; return ret; } else if (PyTypeNum_ISUSERDEF(type)) { ret = userdescrs[type - PyArray_USERDEF]; } else { int num = PyArray_NTYPES; if (type < _MAX_LETTER) { num = (int) _letter_to_num[type]; } if (num >= PyArray_NTYPES) { ret = NULL; } else { ret = _builtin_descrs[num]; } } if (ret == NULL) { PyErr_SetString(PyExc_ValueError, "Invalid data-type for array"); } else { Py_INCREF(ret); } return ret; } Yet it is unchecked in several places: static int PyArray_CanCastSafely(int fromtype, int totype) { PyArray_Descr *from, *to; register int felsize, telsize; if (fromtype == totype) return 1; if (fromtype == PyArray_BOOL) return 1; if (totype == PyArray_BOOL) return 0; if (totype == PyArray_OBJECT || totype == PyArray_VOID) return 1; if (fromtype == PyArray_OBJECT || fromtype == PyArray_VOID) return 0; from = PyArray_DescrFromType(fromtype); /* * cancastto is a PyArray_NOTYPE terminated C-int-array of types that * the data-type can be cast to safely. */ if (from->f->cancastto) { int *curtype; curtype = from->f->cancastto; while (*curtype != PyArray_NOTYPE) { if (*curtype++ == totype) return 1; } } if (PyTypeNum_ISUSERDEF(totype)) return 0; to = PyArray_DescrFromType(totype); telsize = to->elsize; felsize = from->elsize; Py_DECREF(from); Py_DECREF(to); switch(fromtype) { case PyArray_BYTE: case PyArray_SHORT: case PyArray_INT: case PyArray_LONG: case PyArray_LONGLONG: if (PyTypeNum_ISINTEGER(totype)) { if (PyTypeNum_ISUNSIGNED(totype)) { return 0; } else { return (telsize >= felsize); } } else if (PyTypeNum_ISFLOAT(totype)) { if (felsize < 8) return (telsize > felsize); else return (telsize >= felsize); } else if (PyTypeNum_ISCOMPLEX(totype)) { if (felsize < 8) return ((telsize >> 1) > felsize); else return ((telsize >> 1) >= felsize); } else return totype > fromtype; case PyArray_UBYTE: case PyArray_USHORT: case PyArray_UINT: case PyArray_ULONG: case PyArray_ULONGLONG: if (PyTypeNum_ISINTEGER(totype)) { if (PyTypeNum_ISSIGNED(totype)) { return (telsize > felsize); } else { return (telsize >= felsize); } } else if (PyTypeNum_ISFLOAT(totype)) { if (felsize < 8) return (telsize > felsize); else return (telsize >= felsize); } else if (PyTypeNum_ISCOMPLEX(totype)) { if (felsize < 8) return ((telsize >> 1) > felsize); else return ((telsize >> 1) >= felsize); } else return totype > fromtype; case PyArray_FLOAT: case PyArray_DOUBLE: case PyArray_LONGDOUBLE: if (PyTypeNum_ISCOMPLEX(totype)) return ((telsize >> 1) >= felsize); else return (totype > fromtype); case PyArray_CFLOAT: case PyArray_CDOUBLE: case PyArray_CLONGDOUBLE: return (totype > fromtype); case PyArray_STRING: case PyArray_UNICODE: return (totype > fromtype); default: return 0; } } Furthermore, the last function can fail, but doesn't seem to have an error return. What is the best way to go about cleaning this up? Chuck
PyArray_DescrFromType can return NULL Yah, you noticed ;)
Yet it is unchecked in several places: Pity about that. Easy enough to fix though -- just don't lose track of ref counts. In fact, I've already submitted a patch to this function (but not addressing this issue).
static int PyArray_CanCastSafely(int fromtype, int totype) { PyArray_Descr *from, *to; register int felsize, telsize;
if (fromtype == totype) return 1; if (fromtype == PyArray_BOOL) return 1; if (totype == PyArray_BOOL) return 0; if (totype == PyArray_OBJECT || totype == PyArray_VOID) return 1; if (fromtype == PyArray_OBJECT || fromtype == PyArray_VOID) return 0;
from = PyArray_DescrFromType(fromtype); if (from == NULL) return 0;
/* * cancastto is a PyArray_NOTYPE terminated C-int-array of types that * the data-type can be cast to safely. */ if (from->f->cancastto) { int *curtype; curtype = from->f->cancastto; while (*curtype != PyArray_NOTYPE) { if (*curtype++ == totype) return 1; } } if (PyTypeNum_ISUSERDEF(totype)) return 0;
to = PyArray_DescrFromType(totype);
if (to == NULL) { Py_DECREF(from); return 0; }
telsize = to->elsize; felsize = from->elsize; Py_DECREF(from); Py_DECREF(to); ... }
Furthermore, the last function can fail, but doesn't seem to have an error return. What is the best way to go about cleaning this up?
Given the question the function is asking, returning false seems good enough for "failure".
On Sat, Jul 12, 2008 at 8:42 AM, Michael Abbott <michael@araneidae.co.uk> wrote:
PyArray_DescrFromType can return NULL Yah, you noticed ;)
Yet it is unchecked in several places: Pity about that. Easy enough to fix though -- just don't lose track of ref counts. In fact, I've already submitted a patch to this function (but not addressing this issue).
static int PyArray_CanCastSafely(int fromtype, int totype) { PyArray_Descr *from, *to; register int felsize, telsize;
if (fromtype == totype) return 1; if (fromtype == PyArray_BOOL) return 1; if (totype == PyArray_BOOL) return 0; if (totype == PyArray_OBJECT || totype == PyArray_VOID) return 1; if (fromtype == PyArray_OBJECT || fromtype == PyArray_VOID) return 0;
from = PyArray_DescrFromType(fromtype); if (from == NULL) return 0;
/* * cancastto is a PyArray_NOTYPE terminated C-int-array of types that * the data-type can be cast to safely. */ if (from->f->cancastto) { int *curtype; curtype = from->f->cancastto; while (*curtype != PyArray_NOTYPE) { if (*curtype++ == totype) return 1; } } if (PyTypeNum_ISUSERDEF(totype)) return 0;
to = PyArray_DescrFromType(totype);
if (to == NULL) { Py_DECREF(from); return 0; }
telsize = to->elsize; felsize = from->elsize; Py_DECREF(from); Py_DECREF(to); ... }
Furthermore, the last function can fail, but doesn't seem to have an
error
return. What is the best way to go about cleaning this up?
Given the question the function is asking, returning false seems good enough for "failure".
Good point, but a memory error may have been set by PyArray_DescrNew. My impression is that the routine was originally intended to return references to static singleton instances, in which case it couldn't fail. I think we need a separate static instance for PyArray_CHARLTR instead of making a copyof a string type and fudging a few members so that it too can't fail. The array indexing for user defined types probably needs bounds checking also, but I'm not sure what to do there. Chuck
On Sat, Jul 12, 2008 at 10:13 AM, Charles R Harris < charlesr.harris@gmail.com> wrote:
On Sat, Jul 12, 2008 at 8:42 AM, Michael Abbott <michael@araneidae.co.uk> wrote:
PyArray_DescrFromType can return NULL Yah, you noticed ;)
Yet it is unchecked in several places: Pity about that. Easy enough to fix though -- just don't lose track of ref counts. In fact, I've already submitted a patch to this function (but not addressing this issue).
static int PyArray_CanCastSafely(int fromtype, int totype) { PyArray_Descr *from, *to; register int felsize, telsize;
if (fromtype == totype) return 1; if (fromtype == PyArray_BOOL) return 1; if (totype == PyArray_BOOL) return 0; if (totype == PyArray_OBJECT || totype == PyArray_VOID) return 1; if (fromtype == PyArray_OBJECT || fromtype == PyArray_VOID) return 0;
from = PyArray_DescrFromType(fromtype); if (from == NULL) return 0;
/* * cancastto is a PyArray_NOTYPE terminated C-int-array of types
that
* the data-type can be cast to safely. */ if (from->f->cancastto) { int *curtype; curtype = from->f->cancastto; while (*curtype != PyArray_NOTYPE) { if (*curtype++ == totype) return 1; } } if (PyTypeNum_ISUSERDEF(totype)) return 0;
to = PyArray_DescrFromType(totype);
if (to == NULL) { Py_DECREF(from); return 0; }
telsize = to->elsize; felsize = from->elsize; Py_DECREF(from); Py_DECREF(to); ... }
Furthermore, the last function can fail, but doesn't seem to have an
error
return. What is the best way to go about cleaning this up?
Given the question the function is asking, returning false seems good enough for "failure".
Good point, but a memory error may have been set by PyArray_DescrNew. My impression is that the routine was originally intended to return references to static singleton instances, in which case it couldn't fail. I think we need a separate static instance for PyArray_CHARLTR instead of making a copy of a string type and fudging a few members so that it too can't fail. The array indexing for user defined types probably needs bounds checking also, but I'm not sure what to do there.
This bit looks hinky, too: else { int num = PyArray_NTYPES; if (type < _MAX_LETTER) { num = (int) _letter_to_num[type]; } if (num >= PyArray_NTYPES) { ret = NULL; } else { ret = _builtin_descrs[num]; } Type shouldn't have alternate meanings. Maybe this is a backwards compatibility thing. We could write a new function that doesn't set any Python errors, leaving that to higher level routines. Chuck
participants (2)
-
Charles R Harris -
Michael Abbott