Skip to content

Commit

Permalink
Fix various issues:
Browse files Browse the repository at this point in the history 
+ fix dangling pointers when fetching Python error text
+ initialize members in overloaded constructors of Exception class
+ implement assignment operator in sub-class
+ move to PyCXX API to simplify handling with reference counting and reading values from the dict
  • Loading branch information
@wwmayer
wwmayer committed May 15, 2017
1 parent 869e42e commit 7d47a72
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 58 deletions.
83 changes: 44 additions & 39 deletions src/Base/Exception.cpp
View file
Expand Up @@ -29,6 +29,7 @@

#include "Exception.h"
#include "Console.h"
#include <CXX/Objects.hxx>

using namespace Base;

Expand All @@ -37,30 +38,34 @@ TYPESYSTEM_SOURCE(Base::Exception,Base::BaseClass);


Exception::Exception(void)
: _line(0)
{
_sErrMsg = "FreeCAD Exception";
}

Exception::Exception(const Exception &inst)
: BaseClass(),_sErrMsg(inst._sErrMsg), _file(inst._file), _line(inst._line), _function(inst._function)
: _sErrMsg(inst._sErrMsg), _file(inst._file),
_line(inst._line), _function(inst._function)
{
}


Exception::Exception(const char * sMessage)
: _sErrMsg(sMessage)
: _sErrMsg(sMessage), _line(0)
{
}

Exception::Exception(const std::string& sMessage)
: _sErrMsg(sMessage)
: _sErrMsg(sMessage), _line(0)
{
}

Exception &Exception::operator=(const Exception &inst)
{
_sErrMsg = inst._sErrMsg;
return *this;
_sErrMsg = inst._sErrMsg;
_file = inst._file;
_line = inst._line;
_function = inst._function;
return *this;
}

const char* Exception::what(void) const throw()
Expand Down Expand Up @@ -100,38 +105,31 @@ void Exception::ReportException (void) const

PyObject * Exception::getPyObject(void)
{
PyObject *edict = PyDict_New();

PyDict_SetItemString(edict, "sclassname", PyString_FromString(typeid(*this).name()));
PyDict_SetItemString(edict, "sErrMsg", PyString_FromString(this->getMessage().c_str()));
PyDict_SetItemString(edict, "sfile", PyString_FromString(this->getFile().c_str()));
PyDict_SetItemString(edict, "iline", PyInt_FromLong(this->getLine()));
PyDict_SetItemString(edict, "sfunction", PyString_FromString(this->getFunction().c_str()));
PyDict_SetItemString(edict, "swhat", PyString_FromString(this->what()));

return edict;
Py::Dict edict;
edict.setItem("sclassname", Py::String(typeid(*this).name()));
edict.setItem("sErrMsg", Py::String(this->getMessage()));
edict.setItem("sfile", Py::String(this->getFile()));
edict.setItem("iline", Py::Long(this->getLine()));
edict.setItem("sfunction", Py::String(this->getFunction()));
edict.setItem("swhat", Py::String(this->what()));
return Py::new_reference_to(edict);
}

void Exception::setPyObject( PyObject * pydict)
{
if(pydict!=NULL) {
PyObject *pystring;

pystring = PyDict_GetItemString(pydict,"sfile");
if(pystring!=NULL)
_file = std::string(PyString_AsString(pystring));
if (pydict!=NULL) {
Py::Dict edict(pydict);
if (edict.hasKey("sfile"))
_file = static_cast<std::string>(Py::String(edict.getItem("sfile")));

pystring = PyDict_GetItemString(pydict,"sfunction");
if(pystring!=NULL)
_function = std::string(PyString_AsString(pystring));
if (edict.hasKey("sfunction"))
_function = static_cast<std::string>(Py::String(edict.getItem("sfunction")));

pystring = PyDict_GetItemString(pydict,"sErrMsg");
if(pystring!=NULL)
_sErrMsg = std::string(PyString_AsString(pystring));
if (edict.hasKey("sErrMsg"))
_sErrMsg = static_cast<std::string>(Py::String(edict.getItem("sErrMsg")));

pystring = PyDict_GetItemString(pydict,"iline");
if(pystring!=NULL)
_line = PyInt_AsLong(pystring);
if (edict.hasKey("iline"))
_line = static_cast<int>(Py::Long(edict.getItem("iline")));
}
}

Expand Down Expand Up @@ -244,6 +242,14 @@ std::string FileException::getFileName() const
return file.fileName();
}

FileException & FileException::operator=(const FileException &inst)
{
Exception::operator = (inst);
file = inst.file;
_sErrMsgAndFileName = inst._sErrMsgAndFileName;
return *this;
}

const char* FileException::what() const throw()
{
return _sErrMsgAndFileName.c_str();
Expand Down Expand Up @@ -281,20 +287,19 @@ void FileException::ReportException (void) const

PyObject * FileException::getPyObject(void)
{
PyObject *edict = Exception::getPyObject();
PyDict_SetItemString(edict, "filename", PyString_FromString(this->file.fileName().c_str()));

return edict;
Py::Dict edict(Exception::getPyObject(), true);
edict.setItem("filename", Py::String(this->file.fileName()));
return Py::new_reference_to(edict);
}

void FileException::setPyObject( PyObject * pydict)
{
if(pydict!=NULL) {
if (pydict!=NULL) {
Exception::setPyObject(pydict);

PyObject *pystring = PyDict_GetItemString(pydict,"filename");
if(pystring!=NULL)
file = FileInfo(PyString_AsString(pystring));
Py::Dict edict(pydict);
if (edict.hasKey("filename"))
file.setFile(static_cast<std::string>(Py::String(edict.getItem("filename"))));
}
}

Expand Down
2 changes: 2 additions & 0 deletions src/Base/Exception.h
View file
Expand Up @@ -186,6 +186,8 @@ class BaseExport FileException : public Exception
FileException(const FileException &inst);
/// Destruction
virtual ~FileException() throw() {}
/// Assignment operator
FileException &operator=(const FileException &inst);
/// Description of the exception
virtual const char* what() const throw();
/// Report generation
Expand Down
10 changes: 4 additions & 6 deletions src/Base/ExceptionFactory.cpp
View file
Expand Up @@ -23,6 +23,7 @@
#include "PreCompiled.h"

#include "ExceptionFactory.h"
#include <CXX/Objects.hxx>

using namespace Base;

Expand All @@ -46,12 +47,9 @@ void ExceptionFactory::raiseException (PyObject * pydict) const
{
std::string classname;

PyObject *pystring;

pystring = PyDict_GetItemString(pydict,"sclassname");

if(pystring!=NULL) {
classname = std::string(PyString_AsString(pystring));
Py::Dict edict(pydict);
if (edict.hasKey("sclassname")) {
classname = static_cast<std::string>(Py::String(edict.getItem("sclassname")));

std::map<const std::string, AbstractProducer*>::const_iterator pProd;

Expand Down
19 changes: 9 additions & 10 deletions src/Base/Interpreter.cpp
View file
Expand Up @@ -87,22 +87,21 @@ PyException::~PyException() throw()
void PyException::ThrowException(void)
{
PyException myexcp = PyException();

if(PP_PyDict_Object!=NULL) {

PyObject *pystring;

pystring = PyDict_GetItemString(PP_PyDict_Object,"sclassname");
PyGILStateLocker locker;
if (PP_PyDict_Object!=NULL) {
// delete the Python dict upon destruction of edict
Py::Dict edict(PP_PyDict_Object, true);
PP_PyDict_Object = 0;

if(pystring==NULL)
if (!edict.hasKey("sclassname"))
throw myexcp;

std::string exceptionname = std::string(PyString_AsString(pystring));

if(!Base::ExceptionFactory::Instance().CanProduce(exceptionname.c_str()))
std::string exceptionname = static_cast<std::string>(Py::String(edict.getItem("sclassname")));
if (!Base::ExceptionFactory::Instance().CanProduce(exceptionname.c_str()))
throw myexcp;

Base::ExceptionFactory::Instance().raiseException(PP_PyDict_Object);
Base::ExceptionFactory::Instance().raiseException(edict.ptr());
}
else
throw myexcp;
Expand Down
9 changes: 6 additions & 3 deletions src/Base/PyTools.c
View file
Expand Up @@ -259,14 +259,17 @@ void PP_Fetch_Error_Text()
if (errdata != NULL &&
(PyDict_Check(errdata)) ) /* str() increfs */
{
pystring = PyDict_GetItemString(errdata,"swhat");
// PyDict_GetItemString returns a borrowed reference
// so we must make sure not to decrement the reference
PyObject* value = PyDict_GetItemString(errdata,"swhat");

if(pystring!=NULL) {
strncpy(PP_last_error_info, PyString_AsString(pystring), MAX);
if (value!=NULL) {
strncpy(PP_last_error_info, PyString_AsString(value), MAX);
PP_last_error_info[MAX-1] = '\0';
}

pydict = errdata;
Py_INCREF(pydict);
}
else if (errdata != NULL &&
(pystring = PyObject_Str(errdata)) != NULL && /* str(): increfs */
Expand Down

0 comments on commit 7d47a72

Please sign in to comment.