Skip to content

gh-99537: Use Py_SETREF() and Py_CLEAR() functions in C code #99538

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

Closed
wants to merge 2 commits into from
Closed

gh-99537: Use Py_SETREF() and Py_CLEAR() functions in C code #99538

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
447435b
gh-99537: Use Py_CLEAR() function in C code
vstinner Nov 16, 2022
eab04ae
gh-99537: Use Py_SETREF() function in C code
vstinner Nov 16, 2022
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
8 changes: 2 additions & 6 deletions Doc/includes/custom2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1;

if (first) {
tmp = self->first;
self->first = Py_NewRef(first);
Py_XDECREF(tmp);
Py_XSETREF(self->first, Py_NewRef(first));
}
if (last) {
tmp = self->last;
self->last = Py_NewRef(last);
Py_XDECREF(tmp);
Py_XSETREF(self->last, Py_NewRef(last));
}
return 0;
}
Expand Down
16 changes: 4 additions & 12 deletions Doc/includes/custom3.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1;

if (first) {
tmp = self->first;
self->first = Py_NewRef(first);
Py_DECREF(tmp);
Py_SETREF(self->first, Py_NewRef(first));
}
if (last) {
tmp = self->last;
self->last = Py_NewRef(last);
Py_DECREF(tmp);
Py_SETREF(self->last, Py_NewRef(last));
}
return 0;
}
Expand Down Expand Up @@ -87,9 +83,7 @@ Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
"The first attribute value must be a string");
return -1;
}
tmp = self->first;
self->first = Py_NewRef(value);
Py_DECREF(tmp);
Py_SETREF(self->first, Py_NewRef(value));
return 0;
}

Expand All @@ -112,9 +106,7 @@ Custom_setlast(CustomObject *self, PyObject *value, void *closure)
"The last attribute value must be a string");
return -1;
}
tmp = self->last;
self->last = Py_NewRef(value);
Py_DECREF(tmp);
Py_SETREF(self->last, Py_NewRef(value));
return 0;
}

Expand Down
16 changes: 4 additions & 12 deletions Doc/includes/custom4.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1;

if (first) {
tmp = self->first;
self->first = Py_NewRef(first);
Py_DECREF(tmp);
Py_SETREF(self->first, Py_NewRef(first));
}
if (last) {
tmp = self->last;
self->last = Py_NewRef(last);
Py_DECREF(tmp);
Py_SETREF(self->last, Py_NewRef(last));
}
return 0;
}
Expand Down Expand Up @@ -102,9 +98,7 @@ Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
"The first attribute value must be a string");
return -1;
}
Py_INCREF(value);
Py_CLEAR(self->first);
self->first = value;
Py_XSETREF(self->first, Py_NewRef(value));
return 0;
}

Expand All @@ -126,9 +120,7 @@ Custom_setlast(CustomObject *self, PyObject *value, void *closure)
"The last attribute value must be a string");
return -1;
}
Py_INCREF(value);
Py_CLEAR(self->last);
self->last = value;
Py_XSETREF(self->last, Py_NewRef(value));
return 0;
}

Expand Down
3 changes: 1 addition & 2 deletions Modules/_abc.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -624,8 +624,7 @@ _abc__abc_instancecheck_impl(PyObject *module, PyObject *self,

switch (PyObject_IsTrue(result)) {
case -1:
Py_DECREF(result);
result = NULL;
Py_CLEAR(result);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Py_DECREF(var); var=NULL only has a race condition if var is accessible from outside the function, right? I don't think there is any race condition if var is just a local variable.

Copy link
Contributor

@erlend-aasland erlend-aasland Nov 17, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, for those cases you only get increased readability and fewer lines of code.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, the code is only unsafe if the variable is shared between two function calls. It can be a static variable inside the function, a global variable, an object attribute, etc. Here result is a local variable, so the code is safe. Do you think that the commit message should be enhanced?

@erlend-aasland is right, it's also about about increasing readability.

break;
case 0:
Py_DECREF(result);
Expand Down
5 changes: 1 addition & 4 deletions Modules/_collectionsmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1256,7 +1256,6 @@ deque_remove(dequeobject *deque, PyObject *value)
static int
deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v)
{
PyObject *old_value;
block *b;
Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i;

Expand All @@ -1282,9 +1281,7 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v)
while (--n >= 0)
b = b->leftlink;
}
old_value = b->data[i];
b->data[i] = Py_NewRef(v);
Py_DECREF(old_value);
Py_SETREF(b->data[i], Py_NewRef(v));
return 0;
}

Expand Down
16 changes: 5 additions & 11 deletions Modules/_datetimemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1328,8 +1328,7 @@ call_tzname(PyObject *tzinfo, PyObject *tzinfoarg)
PyErr_Format(PyExc_TypeError, "tzinfo.tzname() must "
"return None or a string, not '%s'",
Py_TYPE(result)->tp_name);
Py_DECREF(result);
result = NULL;
Py_CLEAR(result);
}

return result;
Expand Down Expand Up @@ -1849,8 +1848,7 @@ delta_to_microseconds(PyDateTime_Delta *self)
x2 = PyNumber_Multiply(x1, seconds_per_day); /* days in seconds */
if (x2 == NULL)
goto Done;
Py_DECREF(x1);
x1 = NULL;
Py_CLEAR(x1);

/* x2 has days in seconds */
x1 = PyLong_FromLong(GET_TD_SECONDS(self)); /* seconds */
Expand All @@ -1867,8 +1865,7 @@ delta_to_microseconds(PyDateTime_Delta *self)
x1 = PyNumber_Multiply(x3, us_per_second); /* us */
if (x1 == NULL)
goto Done;
Py_DECREF(x3);
x3 = NULL;
Py_CLEAR(x3);

/* x1 has days+seconds in us */
x2 = PyLong_FromLong(GET_TD_MICROSECONDS(self));
Expand Down Expand Up @@ -2038,8 +2035,7 @@ multiply_truedivide_timedelta_float(PyDateTime_Delta *delta, PyObject *floatobj,
goto error;
}
temp = PyNumber_Multiply(pyus_in, PyTuple_GET_ITEM(ratio, op));
Py_DECREF(pyus_in);
pyus_in = NULL;
Py_CLEAR(pyus_in);
if (temp == NULL)
goto error;
pyus_out = divide_nearest(temp, PyTuple_GET_ITEM(ratio, !op));
Expand Down Expand Up @@ -6247,9 +6243,7 @@ datetime_astimezone(PyDateTime_DateTime *self, PyObject *args, PyObject *kw)
}
else {
/* Result is already aware - just replace tzinfo. */
temp = result->tzinfo;
result->tzinfo = Py_NewRef(PyDateTime_TimeZone_UTC);
Py_DECREF(temp);
Py_SETREF(result->tzinfo, Py_NewRef(PyDateTime_TimeZone_UTC));
}

/* Attach new tzinfo and let fromutc() do the rest. */
Expand Down
9 changes: 3 additions & 6 deletions Modules/_elementtree.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -345,8 +345,7 @@ get_attrib_from_keywords(PyObject *kwds)
}
attrib = PyDict_Copy(attrib);
if (attrib && PyDict_DelItem(kwds, attrib_str) < 0) {
Py_DECREF(attrib);
attrib = NULL;
Py_CLEAR(attrib);
}
}
else if (!PyErr_Occurred()) {
Expand Down Expand Up @@ -537,8 +536,7 @@ element_get_text(ElementObject* self)
if (!tmp)
return NULL;
self->text = tmp;
Py_DECREF(res);
res = tmp;
Py_SETREF(res, tmp);
}
}

Expand All @@ -559,8 +557,7 @@ element_get_tail(ElementObject* self)
if (!tmp)
return NULL;
self->tail = tmp;
Py_DECREF(res);
res = tmp;
Py_SETREF(res, tmp);
}
}

Expand Down
3 changes: 1 addition & 2 deletions Modules/_functoolsmodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1242,8 +1242,7 @@ lru_cache_clear_list(lru_list_elem *link)
{
while (link != NULL) {
lru_list_elem *next = link->next;
Py_DECREF(link);
link = next;
Py_SETREF(link, next);
}
}

Expand Down
3 changes: 1 addition & 2 deletions Modules/_io/_iomodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -334,8 +334,7 @@ _io_open_impl(PyObject *module, PyObject *file, const char *mode,
goto error;
result = raw;

Py_DECREF(path_or_fd);
path_or_fd = NULL;
Py_CLEAR(path_or_fd);

modeobj = PyUnicode_FromString(mode);
if (modeobj == NULL)
Expand Down
3 changes: 1 addition & 2 deletions Modules/_io/stringio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -193,8 +193,7 @@ write_str(stringio *self, PyObject *obj)
if (self->writenl) {
PyObject *translated = PyUnicode_Replace(
decoded, &_Py_STR(newline), self->writenl, -1);
Py_DECREF(decoded);
decoded = translated;
Py_SETREF(decoded, translated);
}
if (decoded == NULL)
return -1;
Expand Down
9 changes: 3 additions & 6 deletions Modules/_io/textio.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,8 +320,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *myself,
out = PyUnicode_DATA(modified);
PyUnicode_WRITE(kind, out, 0, '\r');
memcpy(out + kind, PyUnicode_DATA(output), kind * output_len);
Py_DECREF(output);
output = modified; /* output remains ready */
Py_SETREF(output, modified); /* output remains ready */
self->pendingcr = 0;
output_len++;
}
Expand All @@ -336,8 +335,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *myself,
PyObject *modified = PyUnicode_Substring(output, 0, output_len -1);
if (modified == NULL)
goto error;
Py_DECREF(output);
output = modified;
Py_SETREF(output, modified);
self->pendingcr = 1;
}
}
Expand Down Expand Up @@ -865,8 +863,7 @@ _textiowrapper_set_decoder(textio *self, PyObject *codec_info,
self->decoder, self->readtranslate ? Py_True : Py_False, NULL);
if (incrementalDecoder == NULL)
return -1;
Py_CLEAR(self->decoder);
self->decoder = incrementalDecoder;
Py_XSETREF(self->decoder, incrementalDecoder);
}

return 0;
Expand Down
4 changes: 1 addition & 3 deletions Modules/_json.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -709,9 +709,7 @@ _parse_object_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ss
if (memokey == NULL) {
goto bail;
}
Py_INCREF(memokey);
Py_DECREF(key);
key = memokey;
Py_SETREF(key, Py_NewRef(memokey));
idx = next_idx;

/* skip whitespace between key and : delimiter, read :, skip whitespace */
Expand Down
10 changes: 3 additions & 7 deletions Modules/_pickle.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1829,8 +1829,7 @@ get_deep_attribute(PyObject *obj, PyObject *names, PyObject **pparent)
n = PyList_GET_SIZE(names);
for (i = 0; i < n; i++) {
PyObject *name = PyList_GET_ITEM(names, i);
Py_XDECREF(parent);
parent = obj;
Py_XSETREF(parent, obj);
(void)_PyObject_LookupAttr(parent, name, &obj);
if (obj == NULL) {
Py_DECREF(parent);
Expand Down Expand Up @@ -3717,9 +3716,7 @@ save_global(PicklerObject *self, PyObject *obj, PyObject *name)
else {
gen_global:
if (parent == module) {
Py_INCREF(lastname);
Py_DECREF(global_name);
global_name = lastname;
Py_SETREF(global_name, Py_NewRef(lastname));
}
if (self->proto >= 4) {
const char stack_global_op = STACK_GLOBAL;
Expand Down Expand Up @@ -4347,8 +4344,7 @@ save(PicklerObject *self, PyObject *obj, int pers_save)
if (reduce_value != Py_NotImplemented) {
goto reduce;
}
Py_DECREF(reduce_value);
reduce_value = NULL;
Py_CLEAR(reduce_value);
}

if (type == &PyType_Type) {
Expand Down
2 changes: 1 addition & 1 deletion Modules/_scproxy.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ get_proxy_settings(PyObject* Py_UNUSED(mod), PyObject *Py_UNUSED(ignored))
if (v == NULL) goto error;

r = PyDict_SetItemString(result, "exclude_simple", v);
Py_DECREF(v); v = NULL;
Py_CLEAR(v);
if (r == -1) goto error;

anArray = CFDictionaryGetValue(proxyDict,
Expand Down
3 changes: 1 addition & 2 deletions Modules/_sqlite/cursor.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,7 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self)
PyObject *factory = self->row_factory;
PyObject *args[] = { (PyObject *)self, row, };
PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL);
Py_DECREF(row);
row = new_row;
Py_SETREF(row, new_row);
}
return row;
}
Expand Down
3 changes: 1 addition & 2 deletions Modules/_struct.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2164,8 +2164,7 @@ cache_struct_converter(PyObject *module, PyObject *fmt, PyStructObject **ptr)
_structmodulestate *state = get_struct_state(module);

if (fmt == NULL) {
Py_DECREF(*ptr);
*ptr = NULL;
Py_CLEAR(*ptr);
return 1;
}

Expand Down
12 changes: 4 additions & 8 deletions Modules/_testcapimodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -574,8 +574,7 @@ test_long_and_overflow(PyObject *self, PyObject *Py_UNUSED(ignored))
}
temp = PyNumber_Add(num, one);
Py_DECREF(one);
Py_DECREF(num);
num = temp;
Py_SETREF(num, temp);
if (num == NULL)
return NULL;
overflow = 0;
Expand Down Expand Up @@ -618,8 +617,7 @@ test_long_and_overflow(PyObject *self, PyObject *Py_UNUSED(ignored))
}
temp = PyNumber_Subtract(num, one);
Py_DECREF(one);
Py_DECREF(num);
num = temp;
Py_SETREF(num, temp);
if (num == NULL)
return NULL;
overflow = 0;
Expand Down Expand Up @@ -738,8 +736,7 @@ test_long_long_and_overflow(PyObject *self, PyObject *Py_UNUSED(ignored))
}
temp = PyNumber_Add(num, one);
Py_DECREF(one);
Py_DECREF(num);
num = temp;
Py_SETREF(num, temp);
if (num == NULL)
return NULL;
overflow = 0;
Expand Down Expand Up @@ -782,8 +779,7 @@ test_long_long_and_overflow(PyObject *self, PyObject *Py_UNUSED(ignored))
}
temp = PyNumber_Subtract(num, one);
Py_DECREF(one);
Py_DECREF(num);
num = temp;
Py_SETREF(num, temp);
if (num == NULL)
return NULL;
overflow = 0;
Expand Down
3 changes: 1 addition & 2 deletions Modules/_testinternalcapi.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,8 +523,7 @@ set_eval_frame_record(PyObject *self, PyObject *list)
PyErr_SetString(PyExc_TypeError, "argument must be a list");
return NULL;
}
Py_CLEAR(record_list);
record_list = Py_NewRef(list);
Py_XSETREF(record_list, Py_NewRef(list));
_PyInterpreterState_SetEvalFrameFunc(PyInterpreterState_Get(), record_eval);
Py_RETURN_NONE;
}
Expand Down
Loading