From 89be34e0289e17748fb3452bdde37e4dade40804 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 20 May 2020 01:43:05 -0400 Subject: [PATCH 1/6] Correctly initialize kwargs to _image.resample. The Python side seems to always pass the optional arguments, but technically, these should be initialized before calling `PyArg_ParseTupleAndKeywords` in case they aren't. --- src/_image_wrapper.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/_image_wrapper.cpp b/src/_image_wrapper.cpp index de65e90c8238..e4a11c1c2790 100644 --- a/src/_image_wrapper.cpp +++ b/src/_image_wrapper.cpp @@ -120,7 +120,12 @@ image_resample(PyObject *self, PyObject* args, PyObject *kwargs) PyArrayObject *output_array = NULL; PyArrayObject *transform_mesh_array = NULL; + params.interpolation = NEAREST; params.transform_mesh = NULL; + params.resample = false; + params.norm = false; + params.radius = 1.0; + params.alpha = 1.0; const char *kwlist[] = { "input_array", "output_array", "transform", "interpolation", From e5abc170778a29fe47c0c9c697ca6f68e4165d00 Mon Sep 17 00:00:00 2001 From: Antony Lee Date: Wed, 20 May 2020 02:40:07 -0400 Subject: [PATCH 2/6] image: Avoid excess NumPy array refcount twiddling. A new array is always supplied, so just check that it is the right type/shape. Co-authored-by: Elliott Sales de Andrade --- src/_image_wrapper.cpp | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/src/_image_wrapper.cpp b/src/_image_wrapper.cpp index e4a11c1c2790..daf6204fde12 100644 --- a/src/_image_wrapper.cpp +++ b/src/_image_wrapper.cpp @@ -151,9 +151,18 @@ image_resample(PyObject *self, PyObject* args, PyObject *kwargs) goto error; } - output_array = (PyArrayObject *)PyArray_FromAny( - py_output_array, NULL, 2, 3, NPY_ARRAY_C_CONTIGUOUS, NULL); - if (output_array == NULL) { + if (!PyArray_Check(py_output_array)) { + PyErr_SetString(PyExc_ValueError, "output array must be a NumPy array"); + goto error; + } + output_array = (PyArrayObject *)py_output_array; + if (!PyArray_IS_C_CONTIGUOUS(output_array)) { + PyErr_SetString(PyExc_ValueError, "output array must be C-contiguous"); + goto error; + } + if (PyArray_NDIM(output_array) < 2 || PyArray_NDIM(output_array) > 3) { + PyErr_SetString(PyExc_ValueError, + "output array must be 2- or 3-dimensional"); goto error; } @@ -335,11 +344,10 @@ image_resample(PyObject *self, PyObject* args, PyObject *kwargs) Py_DECREF(input_array); Py_XDECREF(transform_mesh_array); - return (PyObject *)output_array; + Py_RETURN_NONE; error: Py_XDECREF(input_array); - Py_XDECREF(output_array); Py_XDECREF(transform_mesh_array); return NULL; } From 949cc292739ab7408c90f8d3032e46e772cfbb51 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 20 May 2020 02:44:16 -0400 Subject: [PATCH 3/6] Make array_view.set return a bool. It only returns 0 or 1, so bool hold a better meaning here. --- src/numpy_cpp.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index d2bb119a21f3..57c1506deb01 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -441,7 +441,7 @@ class array_view : public detail::array_view_accessors return *this; } - int set(PyObject *arr, bool contiguous = false) + bool set(PyObject *arr, bool contiguous = false) { PyArrayObject *tmp; @@ -458,7 +458,7 @@ class array_view : public detail::array_view_accessors tmp = (PyArrayObject *)PyArray_FromObject(arr, type_num_of::value, 0, ND); } if (tmp == NULL) { - return 0; + return false; } if (PyArray_NDIM(tmp) == 0 || PyArray_DIM(tmp, 0) == 0) { @@ -469,7 +469,7 @@ class array_view : public detail::array_view_accessors m_strides = zeros; if (PyArray_NDIM(tmp) == 0 && ND == 0) { m_arr = tmp; - return 1; + return true; } } if (PyArray_NDIM(tmp) != ND) { @@ -478,7 +478,7 @@ class array_view : public detail::array_view_accessors ND, PyArray_NDIM(tmp)); Py_DECREF(tmp); - return 0; + return false; } /* Copy some of the data to the view object for faster access */ @@ -489,7 +489,7 @@ class array_view : public detail::array_view_accessors m_data = (char *)PyArray_BYTES(tmp); } - return 1; + return true; } npy_intp dim(size_t i) const From 8e7aaffe768726182ee27e83c3f0f0026d85f11d Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 20 May 2020 02:58:42 -0400 Subject: [PATCH 4/6] Fix leak of input mesh during image resampling. Fixes #15474. --- src/_image_wrapper.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_image_wrapper.cpp b/src/_image_wrapper.cpp index daf6204fde12..9fe172528b20 100644 --- a/src/_image_wrapper.cpp +++ b/src/_image_wrapper.cpp @@ -86,7 +86,7 @@ _get_transform_mesh(PyObject *py_affine, npy_intp *dims) PyObject *output_mesh = PyObject_CallMethod( py_inverse, (char *)"transform", (char *)"O", - (char *)input_mesh.pyobj(), NULL); + (char *)input_mesh.pyobj_steal(), NULL); Py_DECREF(py_inverse); From 8627f646f31a353e3f1852dfcdd927c5324b0fe7 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 20 May 2020 14:56:25 -0400 Subject: [PATCH 5/6] Replace PyObject_Call* with PyObject_Call*ObjArgs. Also, remove extra `(char*)` casts. --- src/_ttconv.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/_ttconv.cpp b/src/_ttconv.cpp index 6a5ceafa0d2e..dc8122860f28 100644 --- a/src/_ttconv.cpp +++ b/src/_ttconv.cpp @@ -49,7 +49,7 @@ class PythonFileWriter : public TTStreamWriter if (decoded == NULL) { throw py::exception(); } - result = PyObject_CallFunction(_write_method, (char *)"O", decoded); + result = PyObject_CallFunctionObjArgs(_write_method, decoded, NULL); Py_DECREF(decoded); if (!result) { throw py::exception(); From 2d2083c71fbc218416f4c3190ac994d5f5f03001 Mon Sep 17 00:00:00 2001 From: Elliott Sales de Andrade Date: Wed, 20 May 2020 15:38:05 -0400 Subject: [PATCH 6/6] Remove extraneous `(char *)` casts. These functions all take `const char *` already, or return `char *` directly. --- src/_image_wrapper.cpp | 9 +++------ src/mplutils.cpp | 2 +- src/numpy_cpp.h | 4 ++-- src/py_converters.cpp | 8 ++++---- 4 files changed, 10 insertions(+), 13 deletions(-) diff --git a/src/_image_wrapper.cpp b/src/_image_wrapper.cpp index 9fe172528b20..ea4f45a76ea3 100644 --- a/src/_image_wrapper.cpp +++ b/src/_image_wrapper.cpp @@ -67,8 +67,7 @@ _get_transform_mesh(PyObject *py_affine, npy_intp *dims) out_dims[0] = dims[0] * dims[1]; out_dims[1] = 2; - py_inverse = PyObject_CallMethod( - py_affine, (char *)"inverted", (char *)"", NULL); + py_inverse = PyObject_CallMethod(py_affine, "inverted", NULL); if (py_inverse == NULL) { return NULL; } @@ -83,10 +82,8 @@ _get_transform_mesh(PyObject *py_affine, npy_intp *dims) } } - PyObject *output_mesh = - PyObject_CallMethod( - py_inverse, (char *)"transform", (char *)"O", - (char *)input_mesh.pyobj_steal(), NULL); + PyObject *output_mesh = PyObject_CallMethod( + py_inverse, "transform", "O", input_mesh.pyobj_steal()); Py_DECREF(py_inverse); diff --git a/src/mplutils.cpp b/src/mplutils.cpp index bc09db52aa9e..237def047d8c 100644 --- a/src/mplutils.cpp +++ b/src/mplutils.cpp @@ -10,7 +10,7 @@ int add_dict_int(PyObject *dict, const char *key, long val) return 1; } - if (PyDict_SetItemString(dict, (char *)key, valobj)) { + if (PyDict_SetItemString(dict, key, valobj)) { Py_DECREF(valobj); return 1; } diff --git a/src/numpy_cpp.h b/src/numpy_cpp.h index 57c1506deb01..36c763d1584e 100644 --- a/src/numpy_cpp.h +++ b/src/numpy_cpp.h @@ -406,7 +406,7 @@ class array_view : public detail::array_view_accessors Py_XINCREF(arr); m_shape = PyArray_DIMS(m_arr); m_strides = PyArray_STRIDES(m_arr); - m_data = (char *)PyArray_BYTES(m_arr); + m_data = PyArray_BYTES(m_arr); } array_view(npy_intp shape[ND]) : m_arr(NULL), m_shape(NULL), m_strides(NULL), m_data(NULL) @@ -486,7 +486,7 @@ class array_view : public detail::array_view_accessors m_arr = tmp; m_shape = PyArray_DIMS(m_arr); m_strides = PyArray_STRIDES(m_arr); - m_data = (char *)PyArray_BYTES(tmp); + m_data = PyArray_BYTES(tmp); } return true; diff --git a/src/py_converters.cpp b/src/py_converters.cpp index d252da0eda55..ace8332dda76 100644 --- a/src/py_converters.cpp +++ b/src/py_converters.cpp @@ -54,9 +54,9 @@ int convert_from_method(PyObject *obj, const char *name, converter func, void *p { PyObject *value; - value = PyObject_CallMethod(obj, (char *)name, NULL); + value = PyObject_CallMethod(obj, name, NULL); if (value == NULL) { - if (!PyObject_HasAttrString(obj, (char *)name)) { + if (!PyObject_HasAttrString(obj, name)) { PyErr_Clear(); return 1; } @@ -76,9 +76,9 @@ int convert_from_attr(PyObject *obj, const char *name, converter func, void *p) { PyObject *value; - value = PyObject_GetAttrString(obj, (char *)name); + value = PyObject_GetAttrString(obj, name); if (value == NULL) { - if (!PyObject_HasAttrString(obj, (char *)name)) { + if (!PyObject_HasAttrString(obj, name)) { PyErr_Clear(); return 1; }