Skip to content

Commit 1b25e8e

Browse files
committed
Add warning about negative indexes in sq_ass_item.
1 parent 67d5fe9 commit 1b25e8e

2 files changed

Lines changed: 22 additions & 17 deletions

File tree

doc/sphinx/source/new_types.rst

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -938,34 +938,36 @@ Tests are in ``tests/unit/test_c_seqobject.py`` which includes failure modes:
938938
Implementation
939939
--------------
940940

941+
.. warning::
942+
943+
There is an undocumented feature when using `sq_ass_item` from `PyObject_SetItem()`_ and `PyObject_DelItem()`_
944+
when using negative indexes when the negative index is *out of range*.
945+
In this case, before the `sq_ass_item` function is called the index will have had the sequence length added to it.
946+
947+
For example if the sequence length is 3 and the given index is -4 then the index that the `sq_ass_item` function
948+
receives is -1.
949+
If the given index is -5 then the index that the `sq_ass_item` function receives is -2.
950+
951+
Thus the slightly odd code below to fix this problem.
952+
Failing to do this will mean out of range errors will not be detected by the `sq_ass_item` function.
953+
954+
941955
In ``src/cpy/Object/cSeqObject.c``:
942956

943957
.. code-block:: c
944958
945959
static int
946960
SequenceLongObject_sq_ass_item(PyObject *self, Py_ssize_t index, PyObject *value) {
947-
fprintf(
948-
stdout, "%s()#%d: self=%p index=%zd value=%p\n",
949-
__FUNCTION__, __LINE__, (void *) self, index, (void *) value
950-
);
951-
/* This is very weird. */
961+
/* See warning above. */
952962
if (index < 0) {
953-
fprintf(
954-
stdout, "%s()#%d: Fixing index index=%zd to %zd\n", __FUNCTION__, __LINE__,
955-
index, index - SequenceLongObject_sq_length(self)
956-
);
957963
index -= SequenceLongObject_sq_length(self);
958964
}
959-
/* Isn't it? */
965+
960966
Py_ssize_t my_index = index;
961967
if (my_index < 0) {
962968
my_index += SequenceLongObject_sq_length(self);
963969
}
964970
// Corner case example: len(self) == 0 and index < 0
965-
fprintf(
966-
stdout, "%s()#%d: len=%zd index=%zd my_index=%zd\n", __FUNCTION__, __LINE__,
967-
SequenceLongObject_sq_length(self), index, my_index
968-
);
969971
if (my_index < 0 || my_index >= SequenceLongObject_sq_length(self)) {
970972
PyErr_Format(
971973
PyExc_IndexError,
@@ -992,13 +994,11 @@ In ``src/cpy/Object/cSeqObject.c``:
992994
SequenceLongObject *self_as_slo = (SequenceLongObject *) self;
993995
/* Special case: deleting the only item in the array. */
994996
if (self_as_slo->size == 1) {
995-
fprintf(stdout, "%s()#%d: deleting empty index\n", __FUNCTION__, __LINE__);
996997
free(self_as_slo->array_long);
997998
self_as_slo->array_long = NULL;
998999
self_as_slo->size = 0;
9991000
} else {
10001001
/* Delete the value and re-compose the array. */
1001-
fprintf(stdout, "%s()#%d: deleting index=%zd\n", __FUNCTION__, __LINE__, index);
10021002
long *new_array = malloc((self_as_slo->size - 1) * sizeof(long));
10031003
if (!new_array) {
10041004
PyErr_Format(

src/cpy/Object/cSeqObject.c

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,7 +222,12 @@ SequenceLongObject_sq_ass_item(PyObject *self, Py_ssize_t index, PyObject *value
222222
stdout, "%s()#%d: self=%p index=%zd value=%p\n",
223223
__FUNCTION__, __LINE__, (void *) self, index, (void *) value
224224
);
225-
/* This is very weird. */
225+
/* This is very weird.
226+
* When the given index is negative and out of range PyObject_SetItem()
227+
* and PyObject_DelItem() will have *already* added the sequence length
228+
* before calling this function.
229+
* So to get the original out of range negative index we have to *subtract*
230+
* the sequence length. */
226231
if (index < 0) {
227232
fprintf(
228233
stdout, "%s()#%d: Fixing index index=%zd to %zd\n", __FUNCTION__, __LINE__,

0 commit comments

Comments
 (0)