From 7f17bf75a6e6ff7595ce96c5cdf7de1ab0f17c8f Mon Sep 17 00:00:00 2001 From: Matti Picus Date: Wed, 2 Feb 2022 22:46:17 +0200 Subject: [PATCH] ENH: review return values for PyArray_DescrNew (#20960) * ENH: review return value from PyArray_DescrNew* calls * BUG: remove unused variable * BUG: typo * Update numpy/core/src/multiarray/methods.c Co-authored-by: Sebastian Berg * Update numpy/core/src/multiarray/methods.c Co-authored-by: Sebastian Berg * Update numpy/core/src/multiarray/getset.c Co-authored-by: Sebastian Berg * Update numpy/core/src/multiarray/methods.c Co-authored-by: Sebastian Berg * fixes from review * Update numpy/core/src/umath/ufunc_type_resolution.c Co-authored-by: Sebastian Berg * move check to internal function * remove check * Remove unnecessary dealloc The dealloc is now part of the Py_DECREF(ret) and handled there. Doing it here would decref it twice. * MAINT: Remove custom error message (and small cleanup) It is probably not good to call PyObject_GetIter() if dtype is NULL and an error is already in progress... (If we check for it, lets try to do it right.) * Fixup DescrNewFromType `DescrNewFromType` cannot fail in most cases, but if it does, DescrNew does not accept NULL as input. Co-authored-by: Sebastian Berg --- .../src/multiarray/_multiarray_tests.c.src | 4 +-- numpy/core/src/multiarray/arrayobject.c | 3 ++ numpy/core/src/multiarray/buffer.c | 6 ++++ numpy/core/src/multiarray/ctors.c | 24 ++++++++++++++- numpy/core/src/multiarray/descriptor.c | 30 ++++++++++++++----- numpy/core/src/multiarray/dtypemeta.c | 8 +++++ numpy/core/src/multiarray/getset.c | 13 ++++---- numpy/core/src/multiarray/methods.c | 17 +++++++++++ numpy/core/src/multiarray/nditer_constr.c | 11 ++++--- numpy/core/src/multiarray/scalarapi.c | 3 ++ numpy/core/src/multiarray/scalartypes.c.src | 10 +++++-- 11 files changed, 104 insertions(+), 25 deletions(-) diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src index 9486b7cffa5..b7a8b08495f 100644 --- a/numpy/core/src/multiarray/_multiarray_tests.c.src +++ b/numpy/core/src/multiarray/_multiarray_tests.c.src @@ -643,14 +643,12 @@ static PyObject * fromstring_null_term_c_api(PyObject *dummy, PyObject *byte_obj) { char *string; - PyArray_Descr *descr; string = PyBytes_AsString(byte_obj); if (string == NULL) { return NULL; } - descr = PyArray_DescrNewFromType(NPY_FLOAT64); - return PyArray_FromString(string, -1, descr, -1, " "); + return PyArray_FromString(string, -1, NULL, -1, " "); } diff --git a/numpy/core/src/multiarray/arrayobject.c b/numpy/core/src/multiarray/arrayobject.c index f99de2a39b4..292523bbc26 100644 --- a/numpy/core/src/multiarray/arrayobject.c +++ b/numpy/core/src/multiarray/arrayobject.c @@ -1005,6 +1005,9 @@ _strings_richcompare(PyArrayObject *self, PyArrayObject *other, int cmp_op, if (PyArray_ISNOTSWAPPED(self) != PyArray_ISNOTSWAPPED(other)) { /* Cast `other` to the same byte order as `self` (both unicode here) */ PyArray_Descr* unicode = PyArray_DescrNew(PyArray_DESCR(self)); + if (unicode == NULL) { + return NULL; + } unicode->elsize = PyArray_DESCR(other)->elsize; PyObject *new = PyArray_FromAny((PyObject *)other, unicode, 0, 0, 0, NULL); diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c index d10122c4f19..d14f87abb84 100644 --- a/numpy/core/src/multiarray/buffer.c +++ b/numpy/core/src/multiarray/buffer.c @@ -1048,12 +1048,18 @@ _descriptor_from_pep3118_format_fast(char const *s, PyObject **result) } descr = PyArray_DescrFromType(type_num); + if (descr == NULL) { + return 0; + } if (byte_order == '=') { *result = (PyObject*)descr; } else { *result = (PyObject*)PyArray_DescrNewByteorder(descr, byte_order); Py_DECREF(descr); + if (result == NULL) { + return 0; + } } return 1; diff --git a/numpy/core/src/multiarray/ctors.c b/numpy/core/src/multiarray/ctors.c index 78003306afe..2535569a68f 100644 --- a/numpy/core/src/multiarray/ctors.c +++ b/numpy/core/src/multiarray/ctors.c @@ -668,6 +668,9 @@ PyArray_NewFromDescr_int( PyArrayObject_fields *fa; npy_intp nbytes; + if (descr == NULL) { + return NULL; + } if (nd > NPY_MAXDIMS || nd < 0) { PyErr_Format(PyExc_ValueError, "number of dimensions must be within [0, %d]", NPY_MAXDIMS); @@ -1115,6 +1118,9 @@ PyArray_New( return NULL; } PyArray_DESCR_REPLACE(descr); + if (descr == NULL) { + return NULL; + } descr->elsize = itemsize; } new = PyArray_NewFromDescr(subtype, descr, nd, dims, strides, @@ -1140,6 +1146,9 @@ _dtype_from_buffer_3118(PyObject *memoryview) * terminate. */ descr = PyArray_DescrNewFromType(NPY_STRING); + if (descr == NULL) { + return NULL; + } descr->elsize = view->itemsize; } return descr; @@ -3583,6 +3592,10 @@ PyArray_FromFile(FILE *fp, PyArray_Descr *dtype, npy_intp num, char *sep) PyArrayObject *ret; size_t nread = 0; + if (dtype == NULL) { + return NULL; + } + if (PyDataType_REFCHK(dtype)) { PyErr_SetString(PyExc_ValueError, "Cannot read into object array"); @@ -3650,6 +3663,9 @@ PyArray_FromBuffer(PyObject *buf, PyArray_Descr *type, int itemsize; int writeable = 1; + if (type == NULL) { + return NULL; + } if (PyDataType_REFCHK(type)) { PyErr_SetString(PyExc_ValueError, @@ -3857,14 +3873,20 @@ NPY_NO_EXPORT PyObject * PyArray_FromIter(PyObject *obj, PyArray_Descr *dtype, npy_intp count) { PyObject *value; - PyObject *iter = PyObject_GetIter(obj); + PyObject *iter = NULL; PyArrayObject *ret = NULL; npy_intp i, elsize, elcount; char *item, *new_data; + if (dtype == NULL) { + return NULL; + } + + iter = PyObject_GetIter(obj); if (iter == NULL) { goto done; } + if (PyDataType_ISUNSIZED(dtype)) { PyErr_SetString(PyExc_ValueError, "Must specify length when using variable-size data-type."); diff --git a/numpy/core/src/multiarray/descriptor.c b/numpy/core/src/multiarray/descriptor.c index 0c539053c9e..a5cb6a9fcad 100644 --- a/numpy/core/src/multiarray/descriptor.c +++ b/numpy/core/src/multiarray/descriptor.c @@ -1381,6 +1381,9 @@ PyArray_DescrNewFromType(int type_num) PyArray_Descr *new; old = PyArray_DescrFromType(type_num); + if (old == NULL) { + return NULL; + } new = PyArray_DescrNew(old); Py_DECREF(old); return new; @@ -2341,7 +2344,7 @@ arraydescr_new(PyTypeObject *subtype, } PyObject *odescr, *metadata=NULL; - PyArray_Descr *descr, *conv; + PyArray_Descr *conv; npy_bool align = NPY_FALSE; npy_bool copy = NPY_FALSE; npy_bool copied = NPY_FALSE; @@ -2363,9 +2366,10 @@ arraydescr_new(PyTypeObject *subtype, /* Get a new copy of it unless it's already a copy */ if (copy && conv->fields == Py_None) { - descr = PyArray_DescrNew(conv); - Py_DECREF(conv); - conv = descr; + PyArray_DESCR_REPLACE(conv); + if (conv == NULL) { + return NULL; + } copied = NPY_TRUE; } @@ -2375,10 +2379,11 @@ arraydescr_new(PyTypeObject *subtype, * underlying dictionary */ if (!copied) { + PyArray_DESCR_REPLACE(conv); + if (conv == NULL) { + return NULL; + } copied = NPY_TRUE; - descr = PyArray_DescrNew(conv); - Py_DECREF(conv); - conv = descr; } if ((conv->metadata != NULL)) { /* @@ -3047,6 +3052,9 @@ PyArray_DescrNewByteorder(PyArray_Descr *self, char newendian) char endian; new = PyArray_DescrNew(self); + if (new == NULL) { + return NULL; + } endian = new->byteorder; if (endian != NPY_IGNORE) { if (newendian == NPY_SWAP) { @@ -3073,6 +3081,10 @@ PyArray_DescrNewByteorder(PyArray_Descr *self, char newendian) int len, i; newfields = PyDict_New(); + if (newfields == NULL) { + Py_DECREF(new); + return NULL; + } /* make new dictionary with replaced PyArray_Descr Objects */ while (PyDict_Next(self->fields, &pos, &key, &value)) { if (NPY_TITLE_KEY(key, value)) { @@ -3114,6 +3126,10 @@ PyArray_DescrNewByteorder(PyArray_Descr *self, char newendian) Py_DECREF(new->subarray->base); new->subarray->base = PyArray_DescrNewByteorder( self->subarray->base, newendian); + if (new->subarray->base == NULL) { + Py_DECREF(new); + return NULL; + } } return new; } diff --git a/numpy/core/src/multiarray/dtypemeta.c b/numpy/core/src/multiarray/dtypemeta.c index cd489d5e7c9..53f38e8e8b1 100644 --- a/numpy/core/src/multiarray/dtypemeta.c +++ b/numpy/core/src/multiarray/dtypemeta.c @@ -153,6 +153,9 @@ string_discover_descr_from_pyobject( "string to large to store inside array."); } PyArray_Descr *res = PyArray_DescrNewFromType(cls->type_num); + if (res == NULL) { + return NULL; + } res->elsize = (int)itemsize; return res; } @@ -171,10 +174,15 @@ void_discover_descr_from_pyobject( } if (PyBytes_Check(obj)) { PyArray_Descr *descr = PyArray_DescrNewFromType(NPY_VOID); + if (descr == NULL) { + return NULL; + } Py_ssize_t itemsize = PyBytes_Size(obj); if (itemsize > NPY_MAX_INT) { PyErr_SetString(PyExc_TypeError, "byte-like to large to store inside array."); + Py_DECREF(descr); + return NULL; } descr->elsize = (int)itemsize; return descr; diff --git a/numpy/core/src/multiarray/getset.c b/numpy/core/src/multiarray/getset.c index a92ac44b784..fc43bb3fe02 100644 --- a/numpy/core/src/multiarray/getset.c +++ b/numpy/core/src/multiarray/getset.c @@ -714,15 +714,18 @@ _get_part(PyArrayObject *self, int imag) } type = PyArray_DescrFromType(float_type_num); + if (type == NULL) { + return NULL; + } offset = (imag ? type->elsize : 0); if (!PyArray_ISNBO(PyArray_DESCR(self)->byteorder)) { - PyArray_Descr *new; - new = PyArray_DescrNew(type); - new->byteorder = PyArray_DESCR(self)->byteorder; - Py_DECREF(type); - type = new; + Py_SETREF(type, PyArray_DescrNew(type)); + if (type == NULL) { + return NULL; + } + type->byteorder = PyArray_DESCR(self)->byteorder; } ret = (PyArrayObject *)PyArray_NewFromDescrAndBase( Py_TYPE(self), diff --git a/numpy/core/src/multiarray/methods.c b/numpy/core/src/multiarray/methods.c index c31a8292ce3..20a70a37e86 100644 --- a/numpy/core/src/multiarray/methods.c +++ b/numpy/core/src/multiarray/methods.c @@ -1330,6 +1330,10 @@ array_sort(PyArrayObject *self, return NULL; } newd = PyArray_DescrNew(saved); + if (newd == NULL) { + Py_DECREF(new_name); + return NULL; + } Py_DECREF(newd->names); newd->names = new_name; ((PyArrayObject_fields *)self)->descr = newd; @@ -1455,6 +1459,10 @@ array_argsort(PyArrayObject *self, return NULL; } newd = PyArray_DescrNew(saved); + if (newd == NULL) { + Py_DECREF(new_name); + return NULL; + } Py_DECREF(newd->names); newd->names = new_name; ((PyArrayObject_fields *)self)->descr = newd; @@ -1512,6 +1520,10 @@ array_argpartition(PyArrayObject *self, return NULL; } newd = PyArray_DescrNew(saved); + if (newd == NULL) { + Py_DECREF(new_name); + return NULL; + } Py_DECREF(newd->names); newd->names = new_name; ((PyArrayObject_fields *)self)->descr = newd; @@ -2144,6 +2156,11 @@ array_setstate(PyArrayObject *self, PyObject *args) } else { fa->descr = PyArray_DescrNew(typecode); + if (fa->descr == NULL) { + Py_CLEAR(fa->mem_handler); + Py_DECREF(rawdata); + return NULL; + } if (PyArray_DESCR(self)->byteorder == NPY_BIG) { PyArray_DESCR(self)->byteorder = NPY_LITTLE; } diff --git a/numpy/core/src/multiarray/nditer_constr.c b/numpy/core/src/multiarray/nditer_constr.c index bf32e1f6b70..2812aaf3cb5 100644 --- a/numpy/core/src/multiarray/nditer_constr.c +++ b/numpy/core/src/multiarray/nditer_constr.c @@ -1128,13 +1128,12 @@ npyiter_prepare_one_operand(PyArrayObject **op, if (op_flags & NPY_ITER_NBO) { /* Check byte order */ if (!PyArray_ISNBO((*op_dtype)->byteorder)) { - PyArray_Descr *nbo_dtype; - /* Replace with a new descr which is in native byte order */ - nbo_dtype = PyArray_DescrNewByteorder(*op_dtype, NPY_NATIVE); - Py_DECREF(*op_dtype); - *op_dtype = nbo_dtype; - + Py_SETREF(*op_dtype, + PyArray_DescrNewByteorder(*op_dtype, NPY_NATIVE)); + if (*op_dtype == NULL) { + return 0; + } NPY_IT_DBG_PRINT("Iterator: Setting NPY_OP_ITFLAG_CAST " "because of NPY_ITER_NBO\n"); /* Indicate that byte order or alignment needs fixing */ diff --git a/numpy/core/src/multiarray/scalarapi.c b/numpy/core/src/multiarray/scalarapi.c index 564352f1fd3..edbe5955ce9 100644 --- a/numpy/core/src/multiarray/scalarapi.c +++ b/numpy/core/src/multiarray/scalarapi.c @@ -625,6 +625,9 @@ PyArray_DescrFromScalar(PyObject *sc) } if (PyDataType_ISUNSIZED(descr)) { PyArray_DESCR_REPLACE(descr); + if (descr == NULL) { + return NULL; + } type_num = descr->type_num; if (type_num == NPY_STRING) { descr->elsize = PyBytes_GET_SIZE(sc); diff --git a/numpy/core/src/multiarray/scalartypes.c.src b/numpy/core/src/multiarray/scalartypes.c.src index 013526ff0c3..2249fa22bc5 100644 --- a/numpy/core/src/multiarray/scalartypes.c.src +++ b/numpy/core/src/multiarray/scalartypes.c.src @@ -3213,12 +3213,16 @@ void_arrtype_new(PyTypeObject *type, PyObject *args, PyObject *kwds) } ((PyVoidScalarObject *)ret)->obval = destptr; Py_SET_SIZE((PyVoidScalarObject *)ret, (int) memu); - ((PyVoidScalarObject *)ret)->descr = - PyArray_DescrNewFromType(NPY_VOID); - ((PyVoidScalarObject *)ret)->descr->elsize = (int) memu; ((PyVoidScalarObject *)ret)->flags = NPY_ARRAY_BEHAVED | NPY_ARRAY_OWNDATA; ((PyVoidScalarObject *)ret)->base = NULL; + ((PyVoidScalarObject *)ret)->descr = + PyArray_DescrNewFromType(NPY_VOID); + if (((PyVoidScalarObject *)ret)->descr == NULL) { + Py_DECREF(ret); + return NULL; + } + ((PyVoidScalarObject *)ret)->descr->elsize = (int) memu; return ret; }