Skip to content

FIx PySequence_AsVectorCoords and discourage usage #3487

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
+36 −29
Open

FIx PySequence_AsVectorCoords and discourage usage #3487

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 36 additions & 29 deletions src_c/math.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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++) {
Expand Down Expand Up @@ -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;
}
Expand All @@ -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++) {
Expand Down Expand Up @@ -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.");
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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");
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down
Loading