Итак, у вас есть ряд проблем, которые нужно исправить. Я перечислил их все под отдельными заголовками, чтобы вы могли просматривать их по одному.
Всегда возвращаюсь listObj
Когда вы получите ошибку в цикле for, вы goto выдадите метку ошибки, которая все еще возвращала список. Возвращая этот список, вы скрываете, что в вашей функции произошла ошибка. Вы должны всегда возвращать NULL, когда ожидаете, что ваша функция вызовет исключение.
Не увеличивается listObj ref count при возврате
Когда ваша функция вызывается, вы получаете заимствованную ссылку на ваши аргументы. Когда вы возвращаете один из этих аргументов, вы создаете новую ссылку на свой список, и поэтому должен увеличить счетчик ссылок. В противном случае интерпретатор будет иметь счетчик ссылок, который на единицу меньше, чем количество фактических ссылок на объект. Это приведет к ошибке, когда интерпретатор освобождает ваш список, когда есть только 1 ссылка, а не 0! Это может привести к ошибке сегмента или в худшем случае к случайным частям доступа к программе, которая с тех пор была освобождена и выделена для какого-либо другого объекта.
Использует PyObject_SetItem с примитивом
PyObject_SetItem можно использовать с dicts и другим классом, который реализует obj[key] = val. Поэтому вы не можете предоставить ему аргумент типа Py_ssize_t. Вместо этого используйте PyList_SetItem, который принимает в качестве аргумента индекса только Py_ssize_t.
Плохая обработка памяти item и incremented_item
PyObject_SetItem и PyList_SetItem оба обрабатывают уменьшение счетчика ссылок объекта, который уже находился в позиции, которая была установлена. Поэтому нам не нужно беспокоиться об управлении счетчиком ссылок item, поскольку мы работаем только со ссылкой , заимствованной из списка. Эта пара функций также ворует ссылку на incremented_item, поэтому нам не нужно беспокоиться об управлении ее счетчиком ссылок.
Утечка памяти при неверных аргументах
Например, когда вы вызываете вашу функцию с помощью int. Вы создадите новую ссылку на объект int 100, но поскольку вы return NULL, а не goto error, эта ссылка будет потеряна. Таким образом, вы должны обрабатывать такие сценарии по-разному. В моем решении я перемещаю вызов PyLong_FromLong в после проверки arg и типа. Таким образом, мы создаем этот новый * объект только тогда, когда нам гарантировано, что он будет использован.
Рабочий код
Примечание: я удалил операторы goto, так как остался только один, и поэтому было больше смысла выполнять обработку ошибок в этой точке, а не позже.
static PyObject *
testadd_update_list(PyObject *self, PyObject *args)
{
PyObject *listObj = NULL;
PyObject *item = NULL;
PyObject *mult = NULL;
PyObject *incremented_item = NULL;
Py_ssize_t numLines;
if (!PyArg_ParseTuple(args, "O:update_list", &listObj))
{
return NULL;
}
if (!PyList_Check(listObj)) {
PyErr_BadArgument();
return NULL;
}
/* get the number of lines passed to us */
// Don't want to rely on the error checking of this function as it gives a weird stack trace.
// Instead, we use Py_ListCheck() and PyErr_BadArgument() as above. Since list is definitely
// a list now, then PyList_Size will never throw an error, and so we could use
// PyList_GET_SIZE(listObj) instead.
numLines = PyList_Size(listObj);
// only initialise mult here, otherwise the above returns would create a memory leak
mult = PyLong_FromLong(100);
if (mult == NULL) {
return NULL;
}
for (Py_ssize_t i=0; i<numLines; i++) {
// pick the item
// It is possible for this line to raise an error, but our invariants should
// ensure no error is ever raised. `list` is always of type list and `i` is always
// in bounds.
item = PyList_GetItem(listObj, i);
// increment it, and check for type errors or memory errors
incremented_item = PyNumber_Add(item, mult);
if (incremented_item == NULL) {
// ERROR!
Py_DECREF(mult);
return NULL;
}
// update the list item
// We definitely have a list, and our index is in bounds, so we should never see an error
// here.
PyList_SetItem(listObj, i, incremented_item);
// PyList_SetItem steals our reference to incremented_item, and so we must be careful in
// how we handle incremented_item now. Either incremented_item will not be our
// responsibility any more or it is NULL. As such, we can just remove our Py_XDECREF call
}
// success!
// We are returning a *new reference* to listObj. We must increment its ref count as a result!
Py_INCREF(listObj);
Py_DECREF(mult);
return listObj;
}
Сноска:
* PyLong_FromLong(100) на самом деле не создает новый объект, а возвращает новую ссылку на существующий объект. Целые числа с низкими значениями (я думаю, 0 <= i < 128) кэшируются, и этот же объект возвращается при необходимости. Это деталь реализации, которая предназначена для того, чтобы избежать высоких уровней выделения и освобождения целых чисел для небольших значений и, таким образом, повысить производительность Python.