gh-99537: Use Py_SETREF() function in C code (#99656)

Fix potential race condition in code patterns:

* Replace "Py_DECREF(var); var = new;" with "Py_SETREF(var, new);"
* Replace "Py_XDECREF(var); var = new;" with "Py_XSETREF(var, new);"
* Replace "Py_CLEAR(var); var = new;" with "Py_XSETREF(var, new);"

Other changes:

* Replace "old = var; var = new; Py_DECREF(var)"
  with "Py_SETREF(var, new);"
* Replace "old = var; var = new; Py_XDECREF(var)"
  with "Py_XSETREF(var, new);"
* And remove the "old" variable.
This commit is contained in:
Victor Stinner 2022-11-22 14:22:22 +01:00 committed by GitHub
parent 135ec7cefb
commit 7e3f09cad9
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
20 changed files with 37 additions and 89 deletions

View File

@ -42,7 +42,7 @@ static int
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds) Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
{ {
static char *kwlist[] = {"first", "last", "number", NULL}; static char *kwlist[] = {"first", "last", "number", NULL};
PyObject *first = NULL, *last = NULL, *tmp; PyObject *first = NULL, *last = NULL;
if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOi", kwlist, if (!PyArg_ParseTupleAndKeywords(args, kwds, "|OOi", kwlist,
&first, &last, &first, &last,
@ -50,14 +50,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1; return -1;
if (first) { if (first) {
tmp = self->first; Py_XSETREF(self->first, Py_NewRef(first));
self->first = Py_NewRef(first);
Py_XDECREF(tmp);
} }
if (last) { if (last) {
tmp = self->last; Py_XSETREF(self->last, Py_NewRef(last));
self->last = Py_NewRef(last);
Py_XDECREF(tmp);
} }
return 0; return 0;
} }

View File

@ -42,7 +42,7 @@ static int
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds) Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
{ {
static char *kwlist[] = {"first", "last", "number", NULL}; static char *kwlist[] = {"first", "last", "number", NULL};
PyObject *first = NULL, *last = NULL, *tmp; PyObject *first = NULL, *last = NULL;
if (!PyArg_ParseTupleAndKeywords(args, kwds, "|UUi", kwlist, if (!PyArg_ParseTupleAndKeywords(args, kwds, "|UUi", kwlist,
&first, &last, &first, &last,
@ -50,14 +50,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1; return -1;
if (first) { if (first) {
tmp = self->first; Py_SETREF(self->first, Py_NewRef(first));
self->first = Py_NewRef(first);
Py_DECREF(tmp);
} }
if (last) { if (last) {
tmp = self->last; Py_SETREF(self->last, Py_NewRef(last));
self->last = Py_NewRef(last);
Py_DECREF(tmp);
} }
return 0; return 0;
} }
@ -77,7 +73,6 @@ Custom_getfirst(CustomObject *self, void *closure)
static int static int
Custom_setfirst(CustomObject *self, PyObject *value, void *closure) Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
{ {
PyObject *tmp;
if (value == NULL) { if (value == NULL) {
PyErr_SetString(PyExc_TypeError, "Cannot delete the first attribute"); PyErr_SetString(PyExc_TypeError, "Cannot delete the first attribute");
return -1; return -1;
@ -87,9 +82,7 @@ Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
"The first attribute value must be a string"); "The first attribute value must be a string");
return -1; return -1;
} }
tmp = self->first; Py_SETREF(self->first, Py_NewRef(value));
self->first = Py_NewRef(value);
Py_DECREF(tmp);
return 0; return 0;
} }
@ -102,7 +95,6 @@ Custom_getlast(CustomObject *self, void *closure)
static int static int
Custom_setlast(CustomObject *self, PyObject *value, void *closure) Custom_setlast(CustomObject *self, PyObject *value, void *closure)
{ {
PyObject *tmp;
if (value == NULL) { if (value == NULL) {
PyErr_SetString(PyExc_TypeError, "Cannot delete the last attribute"); PyErr_SetString(PyExc_TypeError, "Cannot delete the last attribute");
return -1; return -1;
@ -112,9 +104,7 @@ Custom_setlast(CustomObject *self, PyObject *value, void *closure)
"The last attribute value must be a string"); "The last attribute value must be a string");
return -1; return -1;
} }
tmp = self->last; Py_SETREF(self->last, Py_NewRef(value));
self->last = Py_NewRef(value);
Py_DECREF(tmp);
return 0; return 0;
} }

View File

@ -58,7 +58,7 @@ static int
Custom_init(CustomObject *self, PyObject *args, PyObject *kwds) Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
{ {
static char *kwlist[] = {"first", "last", "number", NULL}; static char *kwlist[] = {"first", "last", "number", NULL};
PyObject *first = NULL, *last = NULL, *tmp; PyObject *first = NULL, *last = NULL;
if (!PyArg_ParseTupleAndKeywords(args, kwds, "|UUi", kwlist, if (!PyArg_ParseTupleAndKeywords(args, kwds, "|UUi", kwlist,
&first, &last, &first, &last,
@ -66,14 +66,10 @@ Custom_init(CustomObject *self, PyObject *args, PyObject *kwds)
return -1; return -1;
if (first) { if (first) {
tmp = self->first; Py_SETREF(self->first, Py_NewRef(first));
self->first = Py_NewRef(first);
Py_DECREF(tmp);
} }
if (last) { if (last) {
tmp = self->last; Py_SETREF(self->last, Py_NewRef(last));
self->last = Py_NewRef(last);
Py_DECREF(tmp);
} }
return 0; return 0;
} }
@ -102,9 +98,7 @@ Custom_setfirst(CustomObject *self, PyObject *value, void *closure)
"The first attribute value must be a string"); "The first attribute value must be a string");
return -1; return -1;
} }
Py_INCREF(value); Py_XSETREF(self->first, Py_NewRef(value));
Py_CLEAR(self->first);
self->first = value;
return 0; return 0;
} }
@ -126,9 +120,7 @@ Custom_setlast(CustomObject *self, PyObject *value, void *closure)
"The last attribute value must be a string"); "The last attribute value must be a string");
return -1; return -1;
} }
Py_INCREF(value); Py_XSETREF(self->last, Py_NewRef(value));
Py_CLEAR(self->last);
self->last = value;
return 0; return 0;
} }

View File

@ -1256,7 +1256,6 @@ deque_remove(dequeobject *deque, PyObject *value)
static int static int
deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v) deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v)
{ {
PyObject *old_value;
block *b; block *b;
Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i; Py_ssize_t n, len=Py_SIZE(deque), halflen=(len+1)>>1, index=i;
@ -1282,9 +1281,7 @@ deque_ass_item(dequeobject *deque, Py_ssize_t i, PyObject *v)
while (--n >= 0) while (--n >= 0)
b = b->leftlink; b = b->leftlink;
} }
old_value = b->data[i]; Py_SETREF(b->data[i], Py_NewRef(v));
b->data[i] = Py_NewRef(v);
Py_DECREF(old_value);
return 0; return 0;
} }

View File

@ -6247,13 +6247,10 @@ datetime_astimezone(PyDateTime_DateTime *self, PyObject *args, PyObject *kw)
} }
else { else {
/* Result is already aware - just replace tzinfo. */ /* Result is already aware - just replace tzinfo. */
temp = result->tzinfo; Py_SETREF(result->tzinfo, Py_NewRef(PyDateTime_TimeZone_UTC));
result->tzinfo = Py_NewRef(PyDateTime_TimeZone_UTC);
Py_DECREF(temp);
} }
/* Attach new tzinfo and let fromutc() do the rest. */ /* Attach new tzinfo and let fromutc() do the rest. */
temp = result->tzinfo;
if (tzinfo == Py_None) { if (tzinfo == Py_None) {
tzinfo = local_timezone(result); tzinfo = local_timezone(result);
if (tzinfo == NULL) { if (tzinfo == NULL) {
@ -6263,8 +6260,7 @@ datetime_astimezone(PyDateTime_DateTime *self, PyObject *args, PyObject *kw)
} }
else else
Py_INCREF(tzinfo); Py_INCREF(tzinfo);
result->tzinfo = tzinfo; Py_SETREF(result->tzinfo, tzinfo);
Py_DECREF(temp);
temp = (PyObject *)result; temp = (PyObject *)result;
result = (PyDateTime_DateTime *) result = (PyDateTime_DateTime *)

View File

@ -537,8 +537,7 @@ element_get_text(ElementObject* self)
if (!tmp) if (!tmp)
return NULL; return NULL;
self->text = tmp; self->text = tmp;
Py_DECREF(res); Py_SETREF(res, tmp);
res = tmp;
} }
} }
@ -559,8 +558,7 @@ element_get_tail(ElementObject* self)
if (!tmp) if (!tmp)
return NULL; return NULL;
self->tail = tmp; self->tail = tmp;
Py_DECREF(res); Py_SETREF(res, tmp);
res = tmp;
} }
} }

View File

@ -1243,8 +1243,7 @@ lru_cache_clear_list(lru_list_elem *link)
{ {
while (link != NULL) { while (link != NULL) {
lru_list_elem *next = link->next; lru_list_elem *next = link->next;
Py_DECREF(link); Py_SETREF(link, next);
link = next;
} }
} }

View File

@ -193,8 +193,7 @@ write_str(stringio *self, PyObject *obj)
if (self->writenl) { if (self->writenl) {
PyObject *translated = PyUnicode_Replace( PyObject *translated = PyUnicode_Replace(
decoded, &_Py_STR(newline), self->writenl, -1); decoded, &_Py_STR(newline), self->writenl, -1);
Py_DECREF(decoded); Py_SETREF(decoded, translated);
decoded = translated;
} }
if (decoded == NULL) if (decoded == NULL)
return -1; return -1;

View File

@ -320,8 +320,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *myself,
out = PyUnicode_DATA(modified); out = PyUnicode_DATA(modified);
PyUnicode_WRITE(kind, out, 0, '\r'); PyUnicode_WRITE(kind, out, 0, '\r');
memcpy(out + kind, PyUnicode_DATA(output), kind * output_len); memcpy(out + kind, PyUnicode_DATA(output), kind * output_len);
Py_DECREF(output); Py_SETREF(output, modified); /* output remains ready */
output = modified; /* output remains ready */
self->pendingcr = 0; self->pendingcr = 0;
output_len++; output_len++;
} }
@ -336,8 +335,7 @@ _PyIncrementalNewlineDecoder_decode(PyObject *myself,
PyObject *modified = PyUnicode_Substring(output, 0, output_len -1); PyObject *modified = PyUnicode_Substring(output, 0, output_len -1);
if (modified == NULL) if (modified == NULL)
goto error; goto error;
Py_DECREF(output); Py_SETREF(output, modified);
output = modified;
self->pendingcr = 1; self->pendingcr = 1;
} }
} }
@ -865,8 +863,7 @@ _textiowrapper_set_decoder(textio *self, PyObject *codec_info,
self->decoder, self->readtranslate ? Py_True : Py_False, NULL); self->decoder, self->readtranslate ? Py_True : Py_False, NULL);
if (incrementalDecoder == NULL) if (incrementalDecoder == NULL)
return -1; return -1;
Py_CLEAR(self->decoder); Py_XSETREF(self->decoder, incrementalDecoder);
self->decoder = incrementalDecoder;
} }
return 0; return 0;

View File

@ -709,9 +709,7 @@ _parse_object_unicode(PyScannerObject *s, PyObject *pystr, Py_ssize_t idx, Py_ss
if (memokey == NULL) { if (memokey == NULL) {
goto bail; goto bail;
} }
Py_INCREF(memokey); Py_SETREF(key, Py_NewRef(memokey));
Py_DECREF(key);
key = memokey;
idx = next_idx; idx = next_idx;
/* skip whitespace between key and : delimiter, read :, skip whitespace */ /* skip whitespace between key and : delimiter, read :, skip whitespace */

View File

@ -1829,8 +1829,7 @@ get_deep_attribute(PyObject *obj, PyObject *names, PyObject **pparent)
n = PyList_GET_SIZE(names); n = PyList_GET_SIZE(names);
for (i = 0; i < n; i++) { for (i = 0; i < n; i++) {
PyObject *name = PyList_GET_ITEM(names, i); PyObject *name = PyList_GET_ITEM(names, i);
Py_XDECREF(parent); Py_XSETREF(parent, obj);
parent = obj;
(void)_PyObject_LookupAttr(parent, name, &obj); (void)_PyObject_LookupAttr(parent, name, &obj);
if (obj == NULL) { if (obj == NULL) {
Py_DECREF(parent); Py_DECREF(parent);
@ -3717,9 +3716,7 @@ save_global(PicklerObject *self, PyObject *obj, PyObject *name)
else { else {
gen_global: gen_global:
if (parent == module) { if (parent == module) {
Py_INCREF(lastname); Py_SETREF(global_name, Py_NewRef(lastname));
Py_DECREF(global_name);
global_name = lastname;
} }
if (self->proto >= 4) { if (self->proto >= 4) {
const char stack_global_op = STACK_GLOBAL; const char stack_global_op = STACK_GLOBAL;

View File

@ -1123,8 +1123,7 @@ pysqlite_cursor_iternext(pysqlite_Cursor *self)
PyObject *factory = self->row_factory; PyObject *factory = self->row_factory;
PyObject *args[] = { (PyObject *)self, row, }; PyObject *args[] = { (PyObject *)self, row, };
PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL); PyObject *new_row = PyObject_Vectorcall(factory, args, 2, NULL);
Py_DECREF(row); Py_SETREF(row, new_row);
row = new_row;
} }
return row; return row;
} }

View File

@ -523,8 +523,7 @@ set_eval_frame_record(PyObject *self, PyObject *list)
PyErr_SetString(PyExc_TypeError, "argument must be a list"); PyErr_SetString(PyExc_TypeError, "argument must be a list");
return NULL; return NULL;
} }
Py_CLEAR(record_list); Py_XSETREF(record_list, Py_NewRef(list));
record_list = Py_NewRef(list);
_PyInterpreterState_SetEvalFrameFunc(PyInterpreterState_Get(), record_eval); _PyInterpreterState_SetEvalFrameFunc(PyInterpreterState_Get(), record_eval);
Py_RETURN_NONE; Py_RETURN_NONE;
} }

View File

@ -1105,8 +1105,7 @@ fromBignumObj(TkappObject *tkapp, Tcl_Obj *value)
PyMem_Free(bytes); PyMem_Free(bytes);
if (res != NULL && bigValue.sign == MP_NEG) { if (res != NULL && bigValue.sign == MP_NEG) {
PyObject *res2 = PyNumber_Negative(res); PyObject *res2 = PyNumber_Negative(res);
Py_DECREF(res); Py_SETREF(res, res2);
res = res2;
} }
mp_clear(&bigValue); mp_clear(&bigValue);
return res; return res;

View File

@ -1074,8 +1074,7 @@ load_data(PyZoneInfo_ZoneInfo *self, PyObject *file_obj)
// that the dstoff is set correctly in that case. // that the dstoff is set correctly in that case.
if (PyObject_IsTrue(tti->dstoff)) { if (PyObject_IsTrue(tti->dstoff)) {
_ttinfo *tti_after = &(self->tzrule_after.std); _ttinfo *tti_after = &(self->tzrule_after.std);
Py_DECREF(tti_after->dstoff); Py_SETREF(tti_after->dstoff, Py_NewRef(tti->dstoff));
tti_after->dstoff = Py_NewRef(tti->dstoff);
} }
} }

View File

@ -1471,8 +1471,7 @@ audioop_ratecv_impl(PyObject *module, Py_buffer *fragment, int width,
len = (Py_ssize_t)(ncp - PyBytes_AsString(str)); len = (Py_ssize_t)(ncp - PyBytes_AsString(str));
rv = PyBytes_FromStringAndSize rv = PyBytes_FromStringAndSize
(PyBytes_AsString(str), len); (PyBytes_AsString(str), len);
Py_DECREF(str); Py_SETREF(str, rv);
str = rv;
if (str == NULL) if (str == NULL)
goto exit; goto exit;
rv = Py_BuildValue("(O(iO))", str, d, samps); rv = Py_BuildValue("(O(iO))", str, d, samps);

View File

@ -980,8 +980,7 @@ _multibytecodec_MultibyteIncrementalEncoder_setstate_impl(MultibyteIncrementalEn
goto errorexit; goto errorexit;
} }
Py_CLEAR(self->pending); Py_XSETREF(self->pending, pending);
self->pending = pending;
memcpy(self->state.c, statebytes+1+statebytes[0], memcpy(self->state.c, statebytes+1+statebytes[0],
sizeof(self->state.c)); sizeof(self->state.c));
@ -1438,8 +1437,7 @@ mbstreamreader_iread(MultibyteStreamReaderObject *self,
memcpy(ctrdata + self->pendingsize, memcpy(ctrdata + self->pendingsize,
PyBytes_AS_STRING(cres), PyBytes_AS_STRING(cres),
PyBytes_GET_SIZE(cres)); PyBytes_GET_SIZE(cres));
Py_DECREF(cres); Py_SETREF(cres, ctr);
cres = ctr;
self->pendingsize = 0; self->pendingsize = 0;
} }

View File

@ -834,8 +834,7 @@ teedataobject_safe_decref(PyObject *obj)
Py_REFCNT(obj) == 1) { Py_REFCNT(obj) == 1) {
PyObject *nextlink = ((teedataobject *)obj)->nextlink; PyObject *nextlink = ((teedataobject *)obj)->nextlink;
((teedataobject *)obj)->nextlink = NULL; ((teedataobject *)obj)->nextlink = NULL;
Py_DECREF(obj); Py_SETREF(obj, nextlink);
obj = nextlink;
} }
Py_XDECREF(obj); Py_XDECREF(obj);
} }

View File

@ -2069,8 +2069,7 @@ factorial_odd_part(unsigned long n)
Py_DECREF(partial); Py_DECREF(partial);
if (tmp == NULL) if (tmp == NULL)
goto error; goto error;
Py_DECREF(inner); Py_SETREF(inner, tmp);
inner = tmp;
/* Now inner is the product of all odd integers j in the range (0, /* Now inner is the product of all odd integers j in the range (0,
n/2**i], giving the inner product in the formula above. */ n/2**i], giving the inner product in the formula above. */
@ -2078,8 +2077,7 @@ factorial_odd_part(unsigned long n)
tmp = PyNumber_Multiply(outer, inner); tmp = PyNumber_Multiply(outer, inner);
if (tmp == NULL) if (tmp == NULL)
goto error; goto error;
Py_DECREF(outer); Py_SETREF(outer, tmp);
outer = tmp;
} }
Py_DECREF(inner); Py_DECREF(inner);
return outer; return outer;

View File

@ -1202,8 +1202,7 @@ path_converter(PyObject *o, void *p)
} }
/* still owns a reference to the original object */ /* still owns a reference to the original object */
Py_DECREF(o); Py_SETREF(o, res);
o = res;
} }
if (is_unicode) { if (is_unicode) {