pythongh-106033: Get rid of new occurrences of PyDict_GetItem and PyObject_HasAttr (pythonGH-106034)

serhiy-storchaka · web-flow · commit 1d33d5378058 · 2023-06-23T20:10:32.000+03:00
These functions are broken by design because they discard any exceptions raised
inside, including MemoryError and KeyboardInterrupt.  They should not be
used in new code.
diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c
@@ -383,14 +383,15 @@ py_digest_by_digestmod(PyObject *module, PyObject *digestmod, enum Py_hash_type
     } else {
         _hashlibstate *state = get_hashlib_state(module);
         // borrowed ref
-        name_obj = PyDict_GetItem(state->constructs, digestmod);
+        name_obj = PyDict_GetItemWithError(state->constructs, digestmod);
     }
     if (name_obj == NULL) {
-        _hashlibstate *state = get_hashlib_state(module);
-        PyErr_Clear();
-        PyErr_Format(
-            state->unsupported_digestmod_error,
-            "Unsupported digestmod %R", digestmod);
+        if (!PyErr_Occurred()) {
+            _hashlibstate *state = get_hashlib_state(module);
+            PyErr_Format(
+                state->unsupported_digestmod_error,
+                "Unsupported digestmod %R", digestmod);
+        }
         return NULL;
     }
 
diff --git a/Modules/_testcapi/code.c b/Modules/_testcapi/code.c
@@ -9,7 +9,7 @@ get_code_extra_index(PyInterpreterState* interp) {
     PyObject *interp_dict = PyInterpreterState_GetDict(interp); // borrowed
     assert(interp_dict);  // real users would handle missing dict... somehow
 
-    PyObject *index_obj = PyDict_GetItemString(interp_dict, key); // borrowed
+    PyObject *index_obj = _PyDict_GetItemStringWithError(interp_dict, key); // borrowed
     Py_ssize_t index = 0;
     if (!index_obj) {
         if (PyErr_Occurred()) {
diff --git a/Objects/exceptions.c b/Objects/exceptions.c
@@ -207,22 +207,21 @@ BaseException_add_note(PyObject *self, PyObject *note)
         return NULL;
     }
 
-    if (!PyObject_HasAttr(self, &_Py_ID(__notes__))) {
-        PyObject *new_notes = PyList_New(0);
-        if (new_notes == NULL) {
+    PyObject *notes;
+    if (_PyObject_LookupAttr(self, &_Py_ID(__notes__), &notes) < 0) {
+        return NULL;
+    }
+    if (notes == NULL) {
+        notes = PyList_New(0);
+        if (notes == NULL) {
             return NULL;
         }
-        if (PyObject_SetAttr(self, &_Py_ID(__notes__), new_notes) < 0) {
-            Py_DECREF(new_notes);
+        if (PyObject_SetAttr(self, &_Py_ID(__notes__), notes) < 0) {
+            Py_DECREF(notes);
             return NULL;
         }
-        Py_DECREF(new_notes);
     }
-    PyObject *notes = PyObject_GetAttr(self, &_Py_ID(__notes__));
-    if (notes == NULL) {
-        return NULL;
-    }
-    if (!PyList_Check(notes)) {
+    else if (!PyList_Check(notes)) {
         Py_DECREF(notes);
         PyErr_SetString(PyExc_TypeError, "Cannot add note: __notes__ is not a list");
         return NULL;
@@ -941,11 +940,11 @@ exceptiongroup_subset(
     PyException_SetContext(eg, PyException_GetContext(orig));
     PyException_SetCause(eg, PyException_GetCause(orig));
 
-    if (PyObject_HasAttr(orig, &_Py_ID(__notes__))) {
-        PyObject *notes = PyObject_GetAttr(orig, &_Py_ID(__notes__));
-        if (notes == NULL) {
-            goto error;
-        }
+    PyObject *notes;
+    if (_PyObject_LookupAttr(orig, &_Py_ID(__notes__), &notes) < 0) {
+        goto error;
+    }
+    if (notes) {
         if (PySequence_Check(notes)) {
             /* Make a copy so the parts have independent notes lists. */
             PyObject *notes_copy = PySequence_List(notes);
diff --git a/Objects/typeobject.c b/Objects/typeobject.c
@@ -1508,11 +1508,14 @@ type_set_annotations(PyTypeObject *type, PyObject *value, void *context)
 static PyObject *
 type_get_type_params(PyTypeObject *type, void *context)
 {
-    PyObject *params = PyDict_GetItem(lookup_tp_dict(type), &_Py_ID(__type_params__));
+    PyObject *params = PyDict_GetItemWithError(lookup_tp_dict(type), &_Py_ID(__type_params__));
 
     if (params) {
         return Py_NewRef(params);
     }
+    if (PyErr_Occurred()) {
+        return NULL;
+    }
 
     return PyTuple_New(0);
 }
diff --git a/Python/pythonrun.c b/Python/pythonrun.c
@@ -1134,15 +1134,13 @@ print_exception_notes(struct exception_print_context *ctx, PyObject *value)
         return 0;
     }
 
-    if (!PyObject_HasAttr(value, &_Py_ID(__notes__))) {
-        return 0;
-    }
-    PyObject *notes = PyObject_GetAttr(value, &_Py_ID(__notes__));
-    if (notes == NULL) {
-        return -1;
+    PyObject *notes;
+    int res = _PyObject_LookupAttr(value, &_Py_ID(__notes__), &notes);
+    if (res <= 0) {
+        return res;
     }
     if (!PySequence_Check(notes) || PyUnicode_Check(notes) || PyBytes_Check(notes)) {
-        int res = 0;
+        res = 0;
         if (write_indented_margin(ctx, f) < 0) {
             res = -1;
         }

Original file line number	Diff line number	Diff line change
`@@ -1508,11 +1508,14 @@ type_set_annotations(PyTypeObject type, PyObject value, void *context)`
`1508`	`1508`	`static PyObject *`
`1509`	`1509`	`type_get_type_params(PyTypeObject type, void context)`
`1510`	`1510`	`{`
`1511`		`- PyObject *params = PyDict_GetItem(lookup_tp_dict(type), &_Py_ID(__type_params__));`
	`1511`	`+ PyObject *params = PyDict_GetItemWithError(lookup_tp_dict(type), &_Py_ID(__type_params__));`
`1512`	`1512`
`1513`	`1513`	`if (params) {`
`1514`	`1514`	`return Py_NewRef(params);`
`1515`	`1515`	`}`
	`1516`	`+ if (PyErr_Occurred()) {`
	`1517`	`+ return NULL;`
	`1518`	`+ }`
`1516`	`1519`
`1517`	`1520`	`return PyTuple_New(0);`
`1518`	`1521`	`}`