Browse Source

general: Don't abuse PyErr_Restore

The intended purpose of this function is to restore an exception
that has already been raised and saved with PyErr_Fetch. It should
not be used to raise new exceptions nor should it be used to clear
the current exception.

The especially egregious example is
`PyErr_Restore(exc_type, nullptr, nullptr);`
as the null value may not be handled correctly.
Sam Edwards 6 years ago
parent
commit
856754a3de

+ 4 - 14
dtool/src/interrogatedb/py_panda.cxx

@@ -156,8 +156,7 @@ PyObject *Dtool_Raise_AssertionError() {
 #else
   PyObject *message = PyString_FromString(notify->get_assert_error_message().c_str());
 #endif
-  Py_INCREF(PyExc_AssertionError);
-  PyErr_Restore(PyExc_AssertionError, message, nullptr);
+  PyErr_SetObject(PyExc_AssertionError, message);
   notify->clear_assert_failed();
   return nullptr;
 }
@@ -166,14 +165,7 @@ PyObject *Dtool_Raise_AssertionError() {
  * Raises a TypeError with the given message, and returns NULL.
  */
 PyObject *Dtool_Raise_TypeError(const char *message) {
-  // PyErr_Restore is what PyErr_SetString would have ended up calling
-  // eventually anyway, so we might as well just get to the point.
-  Py_INCREF(PyExc_TypeError);
-#if PY_MAJOR_VERSION >= 3
-  PyErr_Restore(PyExc_TypeError, PyUnicode_FromString(message), nullptr);
-#else
-  PyErr_Restore(PyExc_TypeError, PyString_FromString(message), nullptr);
-#endif
+  PyErr_SetString(PyExc_TypeError, message);
   return nullptr;
 }
 
@@ -194,8 +186,7 @@ PyObject *Dtool_Raise_ArgTypeError(PyObject *obj, int param, const char *functio
     function_name, param, type_name,
     Py_TYPE(obj)->tp_name);
 
-  Py_INCREF(PyExc_TypeError);
-  PyErr_Restore(PyExc_TypeError, message, nullptr);
+  PyErr_SetObject(PyExc_TypeError, message);
   return nullptr;
 }
 
@@ -214,8 +205,7 @@ PyObject *Dtool_Raise_AttributeError(PyObject *obj, const char *attribute) {
     "'%.100s' object has no attribute '%.200s'",
     Py_TYPE(obj)->tp_name, attribute);
 
-  Py_INCREF(PyExc_AttributeError);
-  PyErr_Restore(PyExc_AttributeError, message, nullptr);
+  PyErr_SetObject(PyExc_AttributeError, message);
   return nullptr;
 }
 

+ 5 - 5
dtool/src/interrogatedb/py_wrappers.cxx

@@ -75,7 +75,7 @@ static PyObject *Dtool_SequenceWrapper_repr(PyObject *self) {
   }
 
   if (len < 0) {
-    PyErr_Restore(nullptr, nullptr, nullptr);
+    PyErr_Clear();
     return Dtool_WrapperBase_repr(self);
   }
 
@@ -422,7 +422,7 @@ static int Dtool_MappingWrapper_contains(PyObject *self, PyObject *key) {
     return 1;
   } else if (_PyErr_OCCURRED() == PyExc_KeyError ||
              _PyErr_OCCURRED() == PyExc_TypeError) {
-    PyErr_Restore(nullptr, nullptr, nullptr);
+    PyErr_Clear();
     return 0;
   } else {
     return -1;
@@ -480,7 +480,7 @@ static PyObject *Dtool_MappingWrapper_get(PyObject *self, PyObject *args) {
   if (value != nullptr) {
     return value;
   } else if (_PyErr_OCCURRED() == PyExc_KeyError) {
-    PyErr_Restore(nullptr, nullptr, nullptr);
+    PyErr_Clear();
     Py_INCREF(defvalue);
     return defvalue;
   } else {
@@ -944,7 +944,7 @@ static PyObject *Dtool_MutableMappingWrapper_pop(PyObject *self, PyObject *args)
       return nullptr;
     }
   } else if (_PyErr_OCCURRED() == PyExc_KeyError) {
-    PyErr_Restore(nullptr, nullptr, nullptr);
+    PyErr_Clear();
     Py_INCREF(defvalue);
     return defvalue;
   } else {
@@ -1044,7 +1044,7 @@ static PyObject *Dtool_MutableMappingWrapper_setdefault(PyObject *self, PyObject
   if (value != nullptr) {
     return value;
   } else if (_PyErr_OCCURRED() == PyExc_KeyError) {
-    PyErr_Restore(nullptr, nullptr, nullptr);
+    PyErr_Clear();
     if (wrap->_setitem_func(wrap->_base._self, key, defvalue) == 0) {
       Py_INCREF(defvalue);
       return defvalue;

+ 4 - 7
panda/src/event/asyncFuture_ext.cxx

@@ -102,7 +102,7 @@ static PyObject *get_done_result(const AsyncFuture *future) {
           if (value != nullptr) {
             return value;
           }
-          PyErr_Restore(nullptr, nullptr, nullptr);
+          PyErr_Clear();
           Py_DECREF(wrap);
         }
       }
@@ -132,8 +132,7 @@ static PyObject *get_done_result(const AsyncFuture *future) {
                                              nullptr, nullptr);
       }
     }
-    Py_INCREF(exc_type);
-    PyErr_Restore(exc_type, nullptr, nullptr);
+    PyErr_SetNone(exc_type);
     return nullptr;
   }
 }
@@ -154,8 +153,7 @@ static PyObject *gen_next(PyObject *self) {
   } else {
     PyObject *result = get_done_result(future);
     if (result != nullptr) {
-      Py_INCREF(PyExc_StopIteration);
-      PyErr_Restore(PyExc_StopIteration, result, nullptr);
+      PyErr_SetObject(PyExc_StopIteration, result);
     }
     return nullptr;
   }
@@ -225,8 +223,7 @@ result(PyObject *timeout) const {
                                                nullptr, nullptr);
         }
       }
-      Py_INCREF(exc_type);
-      PyErr_Restore(exc_type, nullptr, nullptr);
+      PyErr_SetNone(exc_type);
       return nullptr;
     }
   }

+ 1 - 1
panda/src/event/pythonTask.cxx

@@ -539,7 +539,7 @@ do_python_task() {
         result = Py_None;
         Py_INCREF(result);
 #endif
-        PyErr_Restore(nullptr, nullptr, nullptr);
+        PyErr_Clear();
 
         // If we passed a coroutine into the task, eg. something like:
         //   taskMgr.add(my_async_function())