Merge pull request #3487 from pygame-community/ankith26-math-api

FIx PySequence_AsVectorCoords and discourage usage

Author: ankith26

src_c/math.c

@@ -125,8 +125,8 @@ RealNumber_Check(PyObject *obj);
 static double
 PySequence_GetItem_AsDouble(PyObject *seq, Py_ssize_t index);
 static int
-PySequence_AsVectorCoords(PyObject *seq, double *const coords,
-                          const Py_ssize_t size);
+pg_VectorCoordsFromObjOldDontUseInNewCode(PyObject *seq, double *const coords,
+                                          Py_ssize_t size);
 static int
 pgVectorCompatible_Check(PyObject *obj, Py_ssize_t dim);
 static int
@@ -397,13 +397,26 @@ PySequence_GetItem_AsDouble(PyObject *seq, Py_ssize_t index)
     return value;
 }
 
+/* Use pg_VectorCoordsFromObj instead of this function. That does exact dim
+ * checking.
+ * Note that this function sets a python exception on failures */
 static int
-PySequence_AsVectorCoords(PyObject *seq, double *const coords,
-                          const Py_ssize_t size)
+pg_VectorCoordsFromObjOldDontUseInNewCode(PyObject *seq, double *const coords,
+                                          Py_ssize_t size)
 {
     Py_ssize_t i;
 
+    /* This codepath does not do exact size checking, but for compat reasons
+     * we are gonna keep it as it is */
     if (pgVector_Check(seq)) {
+        Py_ssize_t seq_dim = ((pgVector *)seq)->dim;
+        if (size > seq_dim) {
+            /* Prevent undefined behaviour by consistently 0-ing extra dims */
+            for (i = seq_dim; i < size; ++i) {
+                coords[i] = 0.0;
+            }
+            size = seq_dim;
+        }
         memcpy(coords, ((pgVector *)seq)->coords, sizeof(double) * size);
         return 1;
     }
@@ -464,8 +477,7 @@ pgVectorCompatible_Check(PyObject *obj, Py_ssize_t dim)
 // copies vector coordinates into "coords" array of size >= dim
 // managed by caller. Returns 0 if obj is not compatible or an error
 // occurred. If 0 is returned, the error flag will not normally be set.
-// Callers should set error themselves. This function is a combo of
-// pgVectorCompatible_Check and PySequence_AsVectorCoords
+// Callers should set error themselves.
 static int
 pg_VectorCoordsFromObj(PyObject *obj, Py_ssize_t dim, double *const coords)
 {
@@ -1129,7 +1141,7 @@ vector_SetSlice(pgVector *self, Py_ssize_t ilow, Py_ssize_t ihigh, PyObject *v)
     }
 
     len = ihigh - ilow;
-    if (!PySequence_AsVectorCoords(v, new_coords, len)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(v, new_coords, len)) {
         return -1;
     }
 
@@ -1250,7 +1262,8 @@ vector_ass_subscript(pgVector *self, PyObject *key, PyObject *value)
             double seqitems[VECTOR_MAX_SIZE];
             Py_ssize_t cur, i;
 
-            if (!PySequence_AsVectorCoords(value, seqitems, slicelength)) {
+            if (!pg_VectorCoordsFromObjOldDontUseInNewCode(value, seqitems,
+                                                           slicelength)) {
                 return -1;
             }
             for (cur = start, i = 0; i < slicelength; cur += step, i++) {
@@ -1381,7 +1394,7 @@ vector_richcompare(PyObject *o1, PyObject *o2, int op)
         vec = (pgVector *)o2;
         other = o1;
     }
-    if (!pgVectorCompatible_Check(other, vec->dim)) {
+    if (!pg_VectorCoordsFromObj(other, vec->dim, other_coords)) {
         if (op == Py_EQ) {
             Py_RETURN_FALSE;
         }
@@ -1394,10 +1407,6 @@ vector_richcompare(PyObject *o1, PyObject *o2, int op)
         }
     }
 
-    if (!PySequence_AsVectorCoords(other, other_coords, vec->dim)) {
-        return NULL;
-    }
-
     switch (op) {
         case Py_EQ:
             for (i = 0; i < vec->dim; i++) {
@@ -1493,7 +1502,8 @@ static PyObject *
 vector_dot(pgVector *self, PyObject *other)
 {
     double other_coords[VECTOR_MAX_SIZE];
-    if (!PySequence_AsVectorCoords(other, other_coords, self->dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(other, other_coords,
+                                                   self->dim)) {
         return RAISE(PyExc_TypeError,
                      "Cannot perform dot product with this type.");
     }
@@ -1630,7 +1640,8 @@ vector_slerp(pgVector *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "Od:slerp", &other, &t)) {
         return NULL;
     }
-    if (!PySequence_AsVectorCoords(other, other_coords, self->dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(other, other_coords,
+                                                   self->dim)) {
         return RAISE(PyExc_TypeError, "Argument 1 must be a vector.");
     }
     if (fabs(t) > 1) {
@@ -1701,7 +1712,8 @@ vector_lerp(pgVector *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "Od:Vector.lerp", &other, &t)) {
         return NULL;
     }
-    if (!PySequence_AsVectorCoords(other, other_coords, self->dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(other, other_coords,
+                                                   self->dim)) {
         return RAISE(PyExc_TypeError, "Expected Vector as argument 1");
     }
     if (t < 0 || t > 1) {
@@ -1730,7 +1742,8 @@ vector_smoothstep(pgVector *self, PyObject *args)
     if (!PyArg_ParseTuple(args, "Od:Vector.smoothstep", &other, &t)) {
         return NULL;
     }
-    if (!PySequence_AsVectorCoords(other, other_coords, self->dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(other, other_coords,
+                                                   self->dim)) {
         return RAISE(PyExc_TypeError, "Expected Vector as argument 1");
     }
 
@@ -1765,7 +1778,7 @@ _vector_reflect_helper(double *dst_coords, const double *src_coords,
     double norm_coords[VECTOR_MAX_SIZE];
 
     /* normalize the normal */
-    if (!PySequence_AsVectorCoords(normal, norm_coords, dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(normal, norm_coords, dim)) {
         return 0;
     }
 
@@ -1992,7 +2005,8 @@ vector_project_onto(pgVector *self, PyObject *other)
     double a_dot_b;
     double b_dot_b;
 
-    if (!PySequence_AsVectorCoords(other, other_coords, self->dim)) {
+    if (!pg_VectorCoordsFromObjOldDontUseInNewCode(other, other_coords,
+                                                   self->dim)) {
         return RAISE(PyExc_TypeError, "Expected Vector as argument 1");
     }
 
@@ -3795,12 +3809,8 @@ vector_elementwiseproxy_richcompare(PyObject *o1, PyObject *o2, int op)
     }
     dim = vec->dim;
 
-    if (pgVectorCompatible_Check(other, dim)) {
-        double other_coords[VECTOR_MAX_SIZE];
-
-        if (!PySequence_AsVectorCoords(other, other_coords, dim)) {
-            return NULL;
-        }
+    double other_coords[VECTOR_MAX_SIZE];
+    if (pg_VectorCoordsFromObj(other, dim, other_coords)) {
         /* use diff == diff to check for NaN */
         /* TODO: how should NaN be handled with LT/LE/GT/GE? */
         switch (op) {
@@ -3955,11 +3965,8 @@ vector_elementwiseproxy_generic_math(PyObject *o1, PyObject *o2, int op)
         other = (PyObject *)((vector_elementwiseproxy *)other)->vec;
     }
 
-    if (pgVectorCompatible_Check(other, dim)) {
+    if (pg_VectorCoordsFromObj(other, dim, other_coords)) {
         op |= OP_ARG_VECTOR;
-        if (!PySequence_AsVectorCoords(other, other_coords, dim)) {
-            return NULL;
-        }
     }
     else {
         other_value = PyFloat_AsDouble(other);