From f5235a8057efa00fbafbdafb7cf44852e858cdbf Mon Sep 17 00:00:00 2001 From: wmayer Date: Fri, 24 Jun 2022 14:29:50 +0200 Subject: [PATCH] cppcoreguidelines-pro-type-union-access According to https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md using union for type-punning is undefined behaviour. Replace it with std::memcpy --- src/Base/Interpreter.cpp | 3 +- src/Base/PyObjectBase.h | 18 +++++----- src/Base/QuantityPyImp.cpp | 3 +- src/Base/VectorPyImp.cpp | 3 +- src/Gui/CallTips.cpp | 36 ++++++++++---------- src/Mod/Fem/Gui/ViewProviderFemMeshPyImp.cpp | 3 +- src/Mod/Mesh/App/MeshPyImp.cpp | 13 +++---- src/Mod/Part/App/AppPartPy.cpp | 14 ++++---- src/Mod/Part/App/BSplineCurvePyImp.cpp | 3 +- src/Mod/Path/App/CommandPyImp.cpp | 3 +- src/Mod/Points/App/PointsPyImp.cpp | 3 +- src/Mod/Robot/App/WaypointPyImp.cpp | 3 +- 12 files changed, 46 insertions(+), 59 deletions(-) diff --git a/src/Base/Interpreter.cpp b/src/Base/Interpreter.cpp index b3e608eff2..2bacea30cb 100644 --- a/src/Base/Interpreter.cpp +++ b/src/Base/Interpreter.cpp @@ -513,8 +513,7 @@ void InterpreterSingleton::addType(PyTypeObject* Type,PyObject* Module, const ch // This function is responsible for adding inherited slots from a type's base class. if (PyType_Ready(Type) < 0) return; - union PyType_Object pyType = {Type}; - PyModule_AddObject(Module, Name, pyType.o); + PyModule_AddObject(Module, Name, Base::getTypeAsObject(Type)); } void InterpreterSingleton::addPythonPath(const char* Path) diff --git a/src/Base/PyObjectBase.h b/src/Base/PyObjectBase.h index b66c15add8..b4ea857a7c 100644 --- a/src/Base/PyObjectBase.h +++ b/src/Base/PyObjectBase.h @@ -47,6 +47,7 @@ #endif #define slots #include +#include #include "Exception.h" #ifndef PYCXX_PYTHON_2TO3 @@ -90,15 +91,6 @@ #define PyMOD_INIT_FUNC(name) PyMODINIT_FUNC PyInit_##name(void) #define PyMOD_Return(name) return name -/** - * Union to convert from PyTypeObject to PyObject pointer. - */ -union PyType_Object { - PyTypeObject *t; - PyObject *o; -}; - - /*------------------------------ * Basic defines @@ -122,6 +114,14 @@ inline void Assert(int expr, char *msg) // C++ assert }; } +inline PyObject* getTypeAsObject(PyTypeObject* type) { + // See https://en.cppreference.com/w/cpp/string/byte/memcpy + // and https://en.cppreference.com/w/cpp/language/reinterpret_cast + PyObject* obj; + std::memcpy(&obj, &type, sizeof type); + return obj; +} + } /*------------------------------ diff --git a/src/Base/QuantityPyImp.cpp b/src/Base/QuantityPyImp.cpp index 11d5db20d5..7d0cf2eec0 100644 --- a/src/Base/QuantityPyImp.cpp +++ b/src/Base/QuantityPyImp.cpp @@ -595,8 +595,7 @@ Py::Object QuantityPy::getUnit() const void QuantityPy::setUnit(Py::Object arg) { - union PyType_Object pyType = {&(Base::UnitPy::Type)}; - Py::Type UnitType(pyType.o); + Py::Type UnitType(Base::getTypeAsObject(&Base::UnitPy::Type)); if(!arg.isType(UnitType)) throw Py::AttributeError("Not yet implemented"); diff --git a/src/Base/VectorPyImp.cpp b/src/Base/VectorPyImp.cpp index a7db735367..c3f31b13cb 100644 --- a/src/Base/VectorPyImp.cpp +++ b/src/Base/VectorPyImp.cpp @@ -96,8 +96,7 @@ PyObject* VectorPy::__reduce__(PyObject *args) Py::Tuple tuple(2); - union PyType_Object pyType = {&VectorPy::Type}; - Py::Object type(pyType.o); + Py::Object type(Base::getTypeAsObject(&Base::VectorPy::Type)); tuple.setItem(0, type); Base::Vector3d v = this->value(); diff --git a/src/Gui/CallTips.cpp b/src/Gui/CallTips.cpp index 5ff528137d..541a2ce0f2 100644 --- a/src/Gui/CallTips.cpp +++ b/src/Gui/CallTips.cpp @@ -64,7 +64,7 @@ public: : _var(var), _saveVal(var) { var = tmpVal; } - ~Temporary( void ) + ~Temporary( ) { _var = _saveVal; } private: @@ -255,11 +255,11 @@ QMap CallTipsList::extractTips(const QString& context) const //Py::Object type = obj.type(); Py::Object type(PyObject_Type(obj.ptr()), true); Py::Object inst = obj; // the object instance - union PyType_Object typeobj = {&Base::PyObjectBase::Type}; - union PyType_Object typedoc = {&App::DocumentObjectPy::Type}; - union PyType_Object basetype = {&PyBaseObject_Type}; + PyObject* typeobj = Base::getTypeAsObject(&Base::PyObjectBase::Type); + PyObject* typedoc = Base::getTypeAsObject(&App::DocumentObjectPy::Type); + PyObject* basetype = Base::getTypeAsObject(&PyBaseObject_Type); - if (PyObject_IsSubclass(type.ptr(), typedoc.o) == 1) { + if (PyObject_IsSubclass(type.ptr(), typedoc) == 1) { // From the template Python object we don't query its type object because there we keep // a list of additional methods that we won't see otherwise. But to get the correct doc // strings we query the type's dict in the class itself. @@ -269,14 +269,14 @@ QMap CallTipsList::extractTips(const QString& context) const obj = type; } } - else if (PyObject_IsSubclass(type.ptr(), typeobj.o) == 1) { + else if (PyObject_IsSubclass(type.ptr(), typeobj) == 1) { obj = type; } - else if (PyObject_IsInstance(obj.ptr(), basetype.o) == 1) { + else if (PyObject_IsInstance(obj.ptr(), basetype) == 1) { // New style class which can be a module, type, list, tuple, int, float, ... // Make sure it's not a type object - union PyType_Object typetype = {&PyType_Type}; - if (PyObject_IsInstance(obj.ptr(), typetype.o) != 1) { + PyObject* typetype = Base::getTypeAsObject(&PyType_Type); + if (PyObject_IsInstance(obj.ptr(), typetype) != 1) { // For wrapped objects with PySide2 use the object, not its type // as otherwise attributes added at runtime won't be listed (e.g. MainWindowPy) QString typestr(QLatin1String(Py_TYPE(obj.ptr())->tp_name)); @@ -290,7 +290,7 @@ QMap CallTipsList::extractTips(const QString& context) const } // If we have an instance of PyObjectBase then determine whether it's valid or not - if (PyObject_IsInstance(inst.ptr(), typeobj.o) == 1) { + if (PyObject_IsInstance(inst.ptr(), typeobj) == 1) { Base::PyObjectBase* baseobj = static_cast(inst.ptr()); const_cast(this)->validObject = baseobj->isValid(); } @@ -303,8 +303,8 @@ QMap CallTipsList::extractTips(const QString& context) const // If we derive from PropertyContainerPy we can search for the properties in the // C++ twin class. - union PyType_Object proptypeobj = {&App::PropertyContainerPy::Type}; - if (PyObject_IsSubclass(type.ptr(), proptypeobj.o) == 1) { + PyObject* proptypeobj = Base::getTypeAsObject(&App::PropertyContainerPy::Type); + if (PyObject_IsSubclass(type.ptr(), proptypeobj) == 1) { // These are the attributes of the instance itself which are NOT accessible by // its type object extractTipsFromProperties(inst, tips); @@ -312,8 +312,8 @@ QMap CallTipsList::extractTips(const QString& context) const // If we derive from App::DocumentPy we have direct access to the objects by their internal // names. So, we add these names to the list, too. - union PyType_Object appdoctypeobj = {&App::DocumentPy::Type}; - if (PyObject_IsSubclass(type.ptr(), appdoctypeobj.o) == 1) { + PyObject* appdoctypeobj = Base::getTypeAsObject(&App::DocumentPy::Type); + if (PyObject_IsSubclass(type.ptr(), appdoctypeobj) == 1) { App::DocumentPy* docpy = (App::DocumentPy*)(inst.ptr()); App::Document* document = docpy->getDocumentPtr(); // Make sure that the C++ object is alive @@ -328,8 +328,8 @@ QMap CallTipsList::extractTips(const QString& context) const // If we derive from Gui::DocumentPy we have direct access to the objects by their internal // names. So, we add these names to the list, too. - union PyType_Object guidoctypeobj = {&Gui::DocumentPy::Type}; - if (PyObject_IsSubclass(type.ptr(), guidoctypeobj.o) == 1) { + PyObject* guidoctypeobj = Base::getTypeAsObject(&Gui::DocumentPy::Type); + if (PyObject_IsSubclass(type.ptr(), guidoctypeobj) == 1) { Gui::DocumentPy* docpy = (Gui::DocumentPy*)(inst.ptr()); if (docpy->getDocumentPtr()) { App::Document* document = docpy->getDocumentPtr()->getDocument(); @@ -383,8 +383,8 @@ void CallTipsList::extractTipsFromObject(Py::Object& obj, Py::List& list, QMapgetViewProviderFemMeshPtr()->resetColorByNodeId(); else { std::map NodeDispMap; - union PyType_Object pyType = { &(Base::VectorPy::Type) }; - Py::Type vType(pyType.o); + Py::Type vType(Base::getTypeAsObject(&Base::VectorPy::Type)); for (Py::Dict::iterator it = arg.begin(); it != arg.end(); ++it) { Py::Long id((*it).first); diff --git a/src/Mod/Mesh/App/MeshPyImp.cpp b/src/Mod/Mesh/App/MeshPyImp.cpp index 6fb2bb66b5..6525da3fcc 100644 --- a/src/Mod/Mesh/App/MeshPyImp.cpp +++ b/src/Mod/Mesh/App/MeshPyImp.cpp @@ -375,8 +375,7 @@ PyObject* MeshPy::crossSections(PyObject *args) return nullptr; Py::Sequence list(obj); - union PyType_Object pyType = {&(Base::VectorPy::Type)}; - Py::Type vType(pyType.o); + Py::Type vType(Base::getTypeAsObject(&Base::VectorPy::Type)); std::vector csPlanes; for (Py::Sequence::iterator it = list.begin(); it != list.end(); ++it) { @@ -655,11 +654,8 @@ PyObject* MeshPy::addFacets(PyObject *args) PyObject *list; if (PyArg_ParseTuple(args, "O!", &PyList_Type, &list)) { Py::List list_f(list); - union PyType_Object pyVType = {&(Base::VectorPy::Type)}; - Py::Type vVType(pyVType.o); - - union PyType_Object pyFType = {&(Mesh::FacetPy::Type)}; - Py::Type vFType(pyFType.o); + Py::Type vVType(Base::getTypeAsObject(&Base::VectorPy::Type)); + Py::Type vFType(Base::getTypeAsObject(&Mesh::FacetPy::Type)); std::vector facets; MeshCore::MeshGeomFacet facet; @@ -724,8 +720,7 @@ PyObject* MeshPy::addFacets(PyObject *args) Py::Tuple tuple(list); Py::List list_v(tuple.getItem(0)); std::vector vertices; - union PyType_Object pyVertType = {&(Base::VectorPy::Type)}; - Py::Type vType(pyVertType.o); + Py::Type vType(Base::getTypeAsObject(&Base::VectorPy::Type)); for (Py::List::iterator it = list_v.begin(); it != list_v.end(); ++it) { if ((*it).isType(vType)) { Base::Vector3d v = static_cast((*it).ptr())->value(); diff --git a/src/Mod/Part/App/AppPartPy.cpp b/src/Mod/Part/App/AppPartPy.cpp index 527394730e..5e961db381 100644 --- a/src/Mod/Part/App/AppPartPy.cpp +++ b/src/Mod/Part/App/AppPartPy.cpp @@ -1593,8 +1593,8 @@ private: double angle=360; PyObject *pPnt=nullptr, *pDir=nullptr, *pCrv; Handle(Geom_Curve) curve; - union PyType_Object defaultType = {&Part::TopoShapeSolidPy::Type}; - PyObject* type = defaultType.o; + PyObject* defaultType = Base::getTypeAsObject(&Part::TopoShapeSolidPy::Type); + PyObject* type = defaultType; do { if (PyArg_ParseTuple(args.ptr(), "O!|dddO!O!O!", &(GeometryPy::Type), &pCrv, @@ -1667,19 +1667,19 @@ private: d.SetCoord(vec.x, vec.y, vec.z); } - union PyType_Object shellType = {&Part::TopoShapeShellPy::Type}; - union PyType_Object faceType = {&Part::TopoShapeFacePy::Type}; + PyObject* shellType = Base::getTypeAsObject(&Part::TopoShapeShellPy::Type); + PyObject* faceType = Base::getTypeAsObject(&Part::TopoShapeFacePy::Type); BRepPrimAPI_MakeRevolution mkRev(gp_Ax2(p,d),curve, vmin, vmax, angle*(M_PI/180)); - if (type == defaultType.o) { + if (type == defaultType) { TopoDS_Shape shape = mkRev.Solid(); return Py::asObject(new TopoShapeSolidPy(new TopoShape(shape))); } - else if (type == shellType.o) { + else if (type == shellType) { TopoDS_Shape shape = mkRev.Shell(); return Py::asObject(new TopoShapeShellPy(new TopoShape(shape))); } - else if (type == faceType.o) { + else if (type == faceType) { TopoDS_Shape shape = mkRev.Face(); return Py::asObject(new TopoShapeFacePy(new TopoShape(shape))); } diff --git a/src/Mod/Part/App/BSplineCurvePyImp.cpp b/src/Mod/Part/App/BSplineCurvePyImp.cpp index fcc7c87500..a892e5eb03 100644 --- a/src/Mod/Part/App/BSplineCurvePyImp.cpp +++ b/src/Mod/Part/App/BSplineCurvePyImp.cpp @@ -99,8 +99,7 @@ PyObject* BSplineCurvePy::__reduce__(PyObject *args) Py::Tuple tuple(2); // type object to create an instance - union PyType_Object pyType = {&BSplineCurvePy::Type}; - Py::Object type(pyType.o); + Py::Object type(Base::getTypeAsObject(&BSplineCurvePy::Type)); tuple.setItem(0, type); // create an argument tuple to create a copy diff --git a/src/Mod/Path/App/CommandPyImp.cpp b/src/Mod/Path/App/CommandPyImp.cpp index 365d60ad93..80c73059c2 100644 --- a/src/Mod/Path/App/CommandPyImp.cpp +++ b/src/Mod/Path/App/CommandPyImp.cpp @@ -231,8 +231,7 @@ Py::Object CommandPy::getPlacement(void) const void CommandPy::setPlacement(Py::Object arg) { - union PyType_Object pyType = {&(Base::PlacementPy::Type)}; - Py::Type PlacementType(pyType.o); + Py::Type PlacementType(Base::getTypeAsObject(&(Base::PlacementPy::Type))); if(arg.isType(PlacementType)) { getCommandPtr()->setFromPlacement( *static_cast((*arg))->getPlacementPtr() ); parameters_copy_dict.clear(); diff --git a/src/Mod/Points/App/PointsPyImp.cpp b/src/Mod/Points/App/PointsPyImp.cpp index 2ba91081d2..b8ae561a70 100644 --- a/src/Mod/Points/App/PointsPyImp.cpp +++ b/src/Mod/Points/App/PointsPyImp.cpp @@ -142,8 +142,7 @@ PyObject* PointsPy::addPoints(PyObject * args) try { Py::Sequence list(obj); - union PyType_Object pyType = {&(Base::VectorPy::Type)}; - Py::Type vType(pyType.o); + Py::Type vType(Base::getTypeAsObject(&Base::VectorPy::Type)); for (Py::Sequence::iterator it = list.begin(); it != list.end(); ++it) { if ((*it).isType(vType)) { diff --git a/src/Mod/Robot/App/WaypointPyImp.cpp b/src/Mod/Robot/App/WaypointPyImp.cpp index 558280aedc..5a6edebce6 100644 --- a/src/Mod/Robot/App/WaypointPyImp.cpp +++ b/src/Mod/Robot/App/WaypointPyImp.cpp @@ -202,8 +202,7 @@ Py::Object WaypointPy::getPos(void) const void WaypointPy::setPos(Py::Object arg) { - union PyType_Object pyType = {&(Base::PlacementPy::Type)}; - Py::Type PlacementType(pyType.o); + Py::Type PlacementType(Base::getTypeAsObject(&(Base::PlacementPy::Type))); if(arg.isType(PlacementType)) getWaypointPtr()->EndPos = *static_cast((*arg))->getPlacementPtr(); }