[Python-Dev] cpython: Tighten-up code in the set iterator to use an entry pointer rather than

[Victor Stinner]

Maybe such issue can be detected if Raymond uses the bug tracker for
code review? I remember many cases where Serhiy and Raymond
collaborated successfully and wrote better code thanks to the code
review.

Victor

2015-07-07 9:42 GMT+02:00 Serhiy Storchaka <storchaka at gmail.com>:
> On 07.07.15 05:03, raymond.hettinger wrote:
>>
>> https://hg.python.org/cpython/rev/c9782a9ac031
>> changeset:   96865:c9782a9ac031
>> user:        Raymond Hettinger <python at rcn.com>
>> date:        Mon Jul 06 19:03:01 2015 -0700
>> summary:
>>    Tighten-up code in the set iterator to use an entry pointer rather than
>> indexing.
>>
>> files:
>>    Objects/setobject.c |  35 ++++++++++++--------------------
>>    1 files changed, 13 insertions(+), 22 deletions(-)
>>
>>
>> diff --git a/Objects/setobject.c b/Objects/setobject.c
>> --- a/Objects/setobject.c
>> +++ b/Objects/setobject.c
>> @@ -766,8 +766,8 @@
>>       PyObject_HEAD
>>       PySetObject *si_set; /* Set to NULL when iterator is exhausted */
>>       Py_ssize_t si_used;
>> -    Py_ssize_t si_pos;
>>       Py_ssize_t len;
>> +    setentry *entry;
>>   } setiterobject;
>>
>>   static void
>> @@ -845,8 +845,6 @@
>>
>>   static PyObject *setiter_iternext(setiterobject *si)
>>   {
>> -    PyObject *key;
>> -    Py_ssize_t i, mask;
>>       setentry *entry;
>>       PySetObject *so = si->si_set;
>>
>> @@ -860,25 +858,18 @@
>>           si->si_used = -1; /* Make this state sticky */
>>           return NULL;
>>       }
>> -
>> -    i = si->si_pos;
>> -    assert(i>=0);
>> -    entry = so->table;
>> -    mask = so->mask;
>> -    while (i <= mask && (entry[i].key == NULL || entry[i].key == dummy))
>> -        i++;
>> -    si->si_pos = i+1;
>> -    if (i > mask)
>> -        goto fail;
>> +    if (si->len <= 0) {
>> +        Py_DECREF(so);
>> +        si->si_set = NULL;
>> +        return NULL;
>> +    }
>> +    entry = si->entry;
>> +    while (entry->key == NULL || entry->key == dummy)
>> +        entry++;
>>       si->len--;
>> -    key = entry[i].key;
>> -    Py_INCREF(key);
>> -    return key;
>> -
>> -fail:
>> -    Py_DECREF(so);
>> -    si->si_set = NULL;
>> -    return NULL;
>> +    si->entry = entry + 1;
>> +    Py_INCREF(entry->key);
>> +    return entry->key;
>>   }
>>
>>   PyTypeObject PySetIter_Type = {
>> @@ -923,8 +914,8 @@
>>       Py_INCREF(so);
>>       si->si_set = so;
>>       si->si_used = so->used;
>> -    si->si_pos = 0;
>>       si->len = so->used;
>> +    si->entry = so->table;
>>       _PyObject_GC_TRACK(si);
>>       return (PyObject *)si;
>>   }
>
>
> What if so->table was reallocated during the iteration, but so->used is left
> the same? This change looks unsafe to me.
>
>
> _______________________________________________
> Python-Dev mailing list
> Python-Dev at python.org
> https://mail.python.org/mailman/listinfo/python-dev
> Unsubscribe:
> https://mail.python.org/mailman/options/python-dev/victor.stinner%40gmail.com