[Numpy-discussion] Missing NULL return checks?

[Charles R Harris]

On Sat, Jul 12, 2008 at 10:13 AM, Charles R Harris <
charlesr.harris at gmail.com> wrote:

>
>
> On Sat, Jul 12, 2008 at 8:42 AM, Michael Abbott <michael at 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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/numpy-discussion/attachments/20080712/864e5170/attachment.html>