bpo-35441: Remove dead and buggy code related to PyList_SetItem(). (GH-11033)

miss-islington · ZackerySpytz · web-flow · commit 8b7d8ac09cc0 · 2018-12-08T06:34:49.000-08:00
In _localemodule.c and selectmodule.c, remove dead code that would cause double decrefs if run. In addition, replace PyList_SetItem() with PyList_SET_ITEM() in cases where a new list is populated and there is no possibility of an error. In addition, check if the list changed size in the loop in array_array_fromlist(). (cherry picked from commit 99d56b5) Co-authored-by: Zackery Spytz <zspytz@gmail.com>
diff --git a/Modules/_localemodule.c b/Modules/_localemodule.c
@@ -70,20 +70,13 @@ copy_grouping(const char* s)
     do {
         i++;
         val = PyLong_FromLong(s[i]);
-        if (!val)
-            break;
-        if (PyList_SetItem(result, i, val)) {
-            Py_DECREF(val);
-            val = NULL;
-            break;
+        if (val == NULL) {
+            Py_DECREF(result);
+            return NULL;
         }
+        PyList_SET_ITEM(result, i, val);
     } while (s[i] != '\0' && s[i] != CHAR_MAX);
 
-    if (!val) {
-        Py_DECREF(result);
-        return NULL;
-    }
-
     return result;
 }
 
diff --git a/Modules/arraymodule.c b/Modules/arraymodule.c
@@ -1544,12 +1544,18 @@ array_array_fromlist(arrayobject *self, PyObject *list)
         if (array_resize(self, old_size + n) == -1)
             return NULL;
         for (i = 0; i < n; i++) {
-            PyObject *v = PyList_GetItem(list, i);
+            PyObject *v = PyList_GET_ITEM(list, i);
             if ((*self->ob_descr->setitem)(self,
                             Py_SIZE(self) - n + i, v) != 0) {
                 array_resize(self, old_size);
                 return NULL;
             }
+            if (n != PyList_GET_SIZE(list)) {
+                PyErr_SetString(PyExc_RuntimeError,
+                                "list changed size during iteration");
+                array_resize(self, old_size);
+                return NULL;
+            }
         }
     }
     Py_RETURN_NONE;
@@ -1574,8 +1580,7 @@ array_array_tolist_impl(arrayobject *self)
         PyObject *v = getarrayitem((PyObject *)self, i);
         if (v == NULL)
             goto error;
-        if (PyList_SetItem(list, i, v) < 0)
-            goto error;
+        PyList_SET_ITEM(list, i, v);
     }
     return list;
 
diff --git a/Modules/readline.c b/Modules/readline.c
@@ -936,8 +936,7 @@ on_completion_display_matches_hook(char **matches,
         s = decode(matches[i+1]);
         if (s == NULL)
             goto error;
-        if (PyList_SetItem(m, i, s) == -1)
-            goto error;
+        PyList_SET_ITEM(m, i, s);
     }
     sub = decode(matches[0]);
     r = PyObject_CallFunction(readlinestate_global->completion_display_matches_hook,
diff --git a/Modules/selectmodule.c b/Modules/selectmodule.c
@@ -652,10 +652,7 @@ poll_poll(pollObject *self, PyObject *args)
             goto error;
         }
         PyTuple_SET_ITEM(value, 1, num);
-        if ((PyList_SetItem(result_list, j, value)) == -1) {
-            Py_DECREF(value);
-            goto error;
-        }
+        PyList_SET_ITEM(result_list, j, value);
         i++;
     }
     return result_list;
@@ -981,10 +978,7 @@ devpoll_poll(devpollObject *self, PyObject *args)
         Py_DECREF(num2);
         if (value == NULL)
             goto error;
-        if ((PyList_SetItem(result_list, i, value)) == -1) {
-            Py_DECREF(value);
-            goto error;
-        }
+        PyList_SET_ITEM(result_list, i, value);
     }
 
     return result_list;
diff --git a/PC/winreg.c b/PC/winreg.c
@@ -757,9 +757,13 @@ Reg2Py(BYTE *retDataBuf, DWORD retDataSize, DWORD typ)
                         PyMem_Free(str);
                         return NULL;
                     }
-                    PyList_SetItem(obData,
-                                   index,
-                                   PyUnicode_FromWideChar(str[index], len));
+                    PyObject *uni = PyUnicode_FromWideChar(str[index], len);
+                    if (uni == NULL) {
+                        Py_DECREF(obData);
+                        PyMem_Free(str);
+                        return NULL;
+                    }
+                    PyList_SET_ITEM(obData, index, uni);
                 }
                 PyMem_Free(str);
 
diff --git a/Python/ceval.c b/Python/ceval.c
@@ -5056,7 +5056,7 @@ getarray(long a[256])
             Py_DECREF(l);
             return NULL;
         }
-        PyList_SetItem(l, i, x);
+        PyList_SET_ITEM(l, i, x);
     }
     for (i = 0; i < 256; i++)
         a[i] = 0;
@@ -5078,7 +5078,7 @@ _Py_GetDXProfile(PyObject *self, PyObject *args)
             Py_DECREF(l);
             return NULL;
         }
-        PyList_SetItem(l, i, x);
+        PyList_SET_ITEM(l, i, x);
     }
     return l;
 #endif
diff --git a/Python/sysmodule.c b/Python/sysmodule.c
@@ -2544,7 +2544,7 @@ makepathobject(const wchar_t *path, wchar_t delim)
             Py_DECREF(v);
             return NULL;
         }
-        PyList_SetItem(v, i, w);
+        PyList_SET_ITEM(v, i, w);
         if (*p == '\0')
             break;
         path = p+1;

Original file line number	Diff line number	Diff line change
`@@ -5056,7 +5056,7 @@ getarray(long a[256])`
`5056`	`5056`	`Py_DECREF(l);`
`5057`	`5057`	`return NULL;`
`5058`	`5058`	`}`
`5059`		`- PyList_SetItem(l, i, x);`
	`5059`	`+ PyList_SET_ITEM(l, i, x);`
`5060`	`5060`	`}`
`5061`	`5061`	`for (i = 0; i < 256; i++)`
`5062`	`5062`	`a[i] = 0;`
`@@ -5078,7 +5078,7 @@ _Py_GetDXProfile(PyObject self, PyObject args)`
`5078`	`5078`	`Py_DECREF(l);`
`5079`	`5079`	`return NULL;`
`5080`	`5080`	`}`
`5081`		`- PyList_SetItem(l, i, x);`
	`5081`	`+ PyList_SET_ITEM(l, i, x);`
`5082`	`5082`	`}`
`5083`	`5083`	`return l;`
`5084`	`5084`	`#endif`
Original file line number	Diff line number	Diff line change
`@@ -2544,7 +2544,7 @@ makepathobject(const wchar_t *path, wchar_t delim)`
`2544`	`2544`	`Py_DECREF(v);`
`2545`	`2545`	`return NULL;`
`2546`	`2546`	`}`
`2547`		`- PyList_SetItem(v, i, w);`
	`2547`	`+ PyList_SET_ITEM(v, i, w);`
`2548`	`2548`	`if (*p == '\0')`
`2549`	`2549`	`break;`
`2550`	`2550`	`path = p+1;`