From ac87f071f5fcf05b6a28bcf4ba7eb965daa6959a Mon Sep 17 00:00:00 2001
From: Matti Picus <matti.picus@gmail.com>
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 <sebastian@sipsolutions.net>

* Update numpy/core/src/multiarray/methods.c

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

* Update numpy/core/src/multiarray/getset.c

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

* Update numpy/core/src/multiarray/methods.c

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

* fixes from review

* Update numpy/core/src/umath/ufunc_type_resolution.c

Co-authored-by: Sebastian Berg <sebastian@sipsolutions.net>

* 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 <sebastian@sipsolutions.net>
---
 numpy/core/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               | 16 ++++++++++++
 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, 103 insertions(+), 25 deletions(-)

diff --git a/numpy/core/src/multiarray/_multiarray_tests.c.src b/numpy/core/src/multiarray/_multiarray_tests.c.src
index 3693762..fd7c1d0 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 3f080d9..4c20fc1 100644
--- a/numpy/core/src/multiarray/arrayobject.c
+++ b/numpy/core/src/multiarray/arrayobject.c
@@ -986,6 +986,9 @@ _strings_richcompare(PyArrayObject *self, PyArrayObject *other, int cmp_op,
         if (PyArray_TYPE(self) == NPY_STRING &&
                 PyArray_DESCR(other)->type_num == NPY_UNICODE) {
             PyArray_Descr* unicode = PyArray_DescrNew(PyArray_DESCR(other));
+            if (unicode == NULL) {
+                return NULL;
+            }
             unicode->elsize = PyArray_DESCR(self)->elsize << 2;
             new = PyArray_FromAny((PyObject *)self, unicode,
                                   0, 0, 0, NULL);
diff --git a/numpy/core/src/multiarray/buffer.c b/numpy/core/src/multiarray/buffer.c
index d10122c..d14f87a 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 7b7f977..6991bba 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);
@@ -1137,6 +1140,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,
@@ -1162,6 +1168,9 @@ _dtype_from_buffer_3118(PyObject *memoryview)
          *       terminate.
          */
         descr = PyArray_DescrNewFromType(NPY_STRING);
+        if (descr == NULL) {
+            return NULL;
+        }
         descr->elsize = view->itemsize;
     }
     return descr;
@@ -3559,6 +3568,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");
@@ -3626,6 +3639,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,
@@ -3833,14 +3849,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 0c53905..a5cb6a9 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 cd489d5..53f38e8 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 a4f972b..d640684 100644
--- a/numpy/core/src/multiarray/getset.c
+++ b/numpy/core/src/multiarray/getset.c
@@ -698,15 +698,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 33f78df..2edbc23 100644
--- a/numpy/core/src/multiarray/methods.c
+++ b/numpy/core/src/multiarray/methods.c
@@ -1337,6 +1337,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;
@@ -1462,6 +1466,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;
@@ -1519,6 +1527,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;
@@ -2150,6 +2161,10 @@ array_setstate(PyArrayObject *self, PyObject *args)
                 }
                 else {
                     fa->descr = PyArray_DescrNew(typecode);
+                    if (fa->descr == NULL) {
+                        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 0b9717a..f82a962 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 564352f..edbe595 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 9077618..af98145 100644
--- a/numpy/core/src/multiarray/scalartypes.c.src
+++ b/numpy/core/src/multiarray/scalartypes.c.src
@@ -3212,12 +3212,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;
     }
 
-- 
1.8.3.1

--- a/numpy/core/src/multiarray/arrayobject.c
+++ b/numpy/core/src/multiarray/arrayobject.c
-- 
1.8.3.1