Skip to content
Permalink
Browse files

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.
  • Loading branch information
CFSworks committed Nov 6, 2019
1 parent 9a7e103 commit 856754a3def33cd8aba6effb52943d272a5db4c1
@@ -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;
}

@@ -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;
@@ -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;
}
}
@@ -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())

0 comments on commit 856754a

Please sign in to comment.
You can’t perform that action at this time.