apply offline comments from Guido

iritkatriel · Mar 16, 2023 · Mar 13, 2023 · Mar 14, 2023 · Mar 14, 2023 · Mar 14, 2023
commit 4333933ee1ce0235bac7ff6dffda504cb69f3570
diff --git a/Include/cpython/pyerrors.h b/Include/cpython/pyerrors.h
@@ -112,8 +112,8 @@ PyAPI_FUNC(PyObject *) _PyErr_FormatFromCause(
 
 /* In exceptions.c */
 
-PyAPI_FUNC(PyObject*) _PyException_AddNote(
-     PyBaseExceptionObject *exc,
+PyAPI_FUNC(int) _PyException_AddNote(
+     PyObject *exc,
      PyObject *note);
 
 /* Helper that attempts to replace the current exception with one of the

diff --git a/Lib/test/test_capi/test_exceptions.py b/Lib/test/test_capi/test_exceptions.py
@@ -174,7 +174,7 @@ class Broken(Exception):
             def __init__(self, *arg):
                  raise ValueError("Broken __init__")
 
-        exc = _testcapi.exc_set_object_fetch(Broken, ('abcd'))
+        exc = _testcapi.exc_set_object_fetch(Broken, 'abcd')
         self.assertIsInstance(exc, ValueError)
         self.assertEqual(exc.__notes__[0],
                          "Normalization failed: type=Broken args='abcd'")
@@ -183,7 +183,7 @@ class BadArg:
             def __repr__(self):
                 raise TypeError('Broken arg type')
 
-        exc = _testcapi.exc_set_object_fetch(Broken, (BadArg()))
+        exc = _testcapi.exc_set_object_fetch(Broken, BadArg())
         self.assertIsInstance(exc, ValueError)
         self.assertEqual(exc.__notes__[0],
                          'Normalization failed: type=Broken args=<unknown>')

@@ -3749,10 +3749,19 @@ _PyExc_Fini(PyInterpreterState *interp)
     _PyExc_FiniTypes(interp);
 }
 
-PyObject *
-_PyException_AddNote(PyBaseExceptionObject *exc, PyObject *note)
+int
+_PyException_AddNote(PyObject *exc, PyObject *note)
 {
-    return BaseException_add_note((PyObject *)exc, note);
+    if (!PyExceptionInstance_Check(exc)) {
+        PyErr_Format(PyExc_TypeError,
+                     "exc must be an exception, not '%s'",
+                     Py_TYPE(exc)->tp_name);
+        return -1;
+    }
+    PyObject *r = BaseException_add_note(exc, note);
+    int res = r == NULL ? -1 : 0;
+    Py_XDECREF(r);
+    return res;
 }
 
 /* Helper to do the equivalent of "raise X from Y" in C, but always using

diff --git a/Python/errors.c b/Python/errors.c
@@ -182,28 +182,27 @@ _PyErr_SetObject(PyThreadState *tstate, PyObject *exception, PyObject *value)
     Py_XINCREF(value);
     if (!is_subclass) {
         /* We must normalize the value right now */
-        PyObject *fixed_value;
 
         /* Issue #23571: functions must not be called with an
             exception set */
         _PyErr_Clear(tstate);
 
-        fixed_value = _PyErr_CreateException(exception, value);
-        Py_XDECREF(value);
+        PyObject *fixed_value = _PyErr_CreateException(exception, value);
         if (fixed_value == NULL) {
             PyObject *exc = _PyErr_GetRaisedException(tstate);
             assert(PyExceptionInstance_Check(exc));
 
             PyObject *note = get_normalization_failure_note(tstate, exception, value);
+            Py_XDECREF(value);
             if (note != NULL) {
-                PyObject *res = _PyException_AddNote((PyBaseExceptionObject*)exc, note);
+                /* ignore errors in _PyException_AddNote - they will be overwritten below */
+                _PyException_AddNote(exc, note);
                 Py_DECREF(note);
-                Py_XDECREF(res);
             }
             _PyErr_SetRaisedException(tstate, exc);
             return;
         }
-
+        Py_XDECREF(value);
         value = fixed_value;
     }