fixes #3844: PVS: The pointer was not released in destructor. A memor…

…y leak is possible.
FreeCAD · Apr 21, 2021 · b7aee7b · b7aee7b
1 parent c4f411a
commit b7aee7b
Showing 1 changed file with 36 additions and 74 deletions.
diff --git a/src/Gui/Command.cpp b/src/Gui/Command.cpp
@@ -135,10 +135,25 @@ using namespace Gui::DockWnd;
 // list of modules already loaded by a command (not issue again for macro cleanness)
 std::set<std::string> alreadyLoadedModule;
 
-CommandBase::CommandBase( const char* sMenu, const char* sToolTip, const char* sWhat,
-                          const char* sStatus, const char* sPixmap, const char* sAcc)
-        : sMenuText(sMenu), sToolTipText(sToolTip), sWhatsThis(sWhat?sWhat:sToolTip),
-        sStatusTip(sStatus?sStatus:sToolTip), sPixmap(sPixmap), sAccel(sAcc), _pcAction(0)
+class StringCache {
+public:
+    static const char* New(const char* str) {
+        using StringList = std::list<std::string>;
+        static StringList strings;
+        strings.emplace_back(str);
+        return strings.back().c_str();
+    }
+};
+
+CommandBase::CommandBase(const char* sMenu, const char* sToolTip, const char* sWhat,
+                         const char* sStatus, const char* sPixmap, const char* sAcc)
+  : sMenuText(sMenu)
+  , sToolTipText(sToolTip)
+  , sWhatsThis(sWhat ? sWhat : sToolTip)
+  , sStatusTip(sStatus ? sStatus : sToolTip)
+  , sPixmap(sPixmap)
+  , sAccel(sAcc)
+  , _pcAction(nullptr)
 {
 }
 
@@ -162,56 +177,32 @@ Action * CommandBase::createAction()
 
 void CommandBase::setMenuText(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sMenuText = _strdup(s);
-#else
-    this->sMenuText = strdup(s);
-#endif
+    this->sMenuText = StringCache::New(s);
 }
 
 void CommandBase::setToolTipText(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sToolTipText = _strdup(s);
-#else
-    this->sToolTipText = strdup(s);
-#endif
+    this->sToolTipText = StringCache::New(s);
 }
 
 void CommandBase::setStatusTip(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sStatusTip = _strdup(s);
-#else
-    this->sStatusTip = strdup(s);
-#endif
+    this->sStatusTip = StringCache::New(s);
 }
 
 void CommandBase::setWhatsThis(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sWhatsThis = _strdup(s);
-#else
-    this->sWhatsThis = strdup(s);
-#endif
+    this->sWhatsThis = StringCache::New(s);
 }
 
 void CommandBase::setPixmap(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sPixmap = _strdup(s);
-#else
-    this->sPixmap = strdup(s);
-#endif
+    this->sPixmap = StringCache::New(s);
 }
 
 void CommandBase::setAccel(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sAccel = _strdup(s);
-#else
-    this->sAccel = strdup(s);
-#endif
+    this->sAccel = StringCache::New(s);
 }
 
 //===========================================================================
@@ -221,7 +212,9 @@ void CommandBase::setAccel(const char* s)
 /* TRANSLATOR Gui::Command */
 
 Command::Command(const char* name)
-        : CommandBase(0), sName(name), sHelpUrl(0)
+    : CommandBase(nullptr)
+    , sName(name)
+    , sHelpUrl(0)
 {
     sAppModule  = "FreeCAD";
     sGroup      = QT_TR_NOOP("Standard");
@@ -578,20 +571,12 @@ std::string Command::getObjectCmd(const App::DocumentObject *obj,
 
 void Command::setAppModuleName(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sAppModule = _strdup(s);
-#else
-    this->sAppModule = strdup(s);
-#endif
+    this->sAppModule = StringCache::New(s);
 }
 
 void Command::setGroupName(const char* s)
 {
-#if defined (_MSC_VER)
-    this->sGroup = _strdup(s);
-#else
-    this->sGroup = strdup(s);
-#endif
+    this->sGroup = StringCache::New(s);
 }
 
 //--------------------------------------------------------------------------
@@ -1073,23 +1058,16 @@ void GroupCommand::setup(Action *pcAction) {
 /* TRANSLATOR Gui::MacroCommand */
 
 MacroCommand::MacroCommand(const char* name, bool system)
-#if defined (_MSC_VER)
-  : Command( _strdup(name) ), systemMacro(system)
-#else
-  : Command( strdup(name) ), systemMacro(system)
-#endif
+  : Command(StringCache::New(name))
+  , systemMacro(system)
 {
     sGroup = QT_TR_NOOP("Macros");
     eType  = 0;
-    sScriptName = 0;
+    sScriptName = nullptr;
 }
 
 MacroCommand::~MacroCommand()
 {
-    free(const_cast<char*>(sName));
-    sName = 0;
-    free(const_cast<char*>(sScriptName));
-    sScriptName = 0;
 }
 
 void MacroCommand::activated(int iMsg)
@@ -1157,11 +1135,7 @@ Action * MacroCommand::createAction(void)
 
 void MacroCommand::setScriptName( const char* s )
 {
-#if defined (_MSC_VER)
-    this->sScriptName = _strdup( s );
-#else
-    this->sScriptName = strdup( s );
-#endif
+    this->sScriptName = StringCache::New(s);
 }
 
 void MacroCommand::load()
@@ -1214,11 +1188,7 @@ void MacroCommand::save()
 //===========================================================================
 
 PythonCommand::PythonCommand(const char* name, PyObject * pcPyCommand, const char* pActivationString)
-#if defined (_MSC_VER)
-  : Command( _strdup(name) )
-#else
-  : Command( strdup(name) )
-#endif
+  : Command(StringCache::New(name))
   ,_pcPyCommand(pcPyCommand)
 {
     if (pActivationString)
@@ -1258,8 +1228,6 @@ PythonCommand::~PythonCommand()
 {
     Base::PyGILStateLocker lock;
     Py_DECREF(_pcPyCommand);
-    free(const_cast<char*>(sName));
-    sName = 0;
 }
 
 const char* PythonCommand::getResource(const char* sName) const
@@ -1449,11 +1417,7 @@ bool PythonCommand::isChecked() const
 //===========================================================================
 
 PythonGroupCommand::PythonGroupCommand(const char* name, PyObject * pcPyCommand)
-#if defined (_MSC_VER)
-  : Command( _strdup(name) )
-#else
-  : Command( strdup(name) )
-#endif
+  : Command(StringCache::New(name))
   ,_pcPyCommand(pcPyCommand)
 {
     sGroup = "Python";
@@ -1488,8 +1452,6 @@ PythonGroupCommand::~PythonGroupCommand()
 {
     Base::PyGILStateLocker lock;
     Py_DECREF(_pcPyCommand);
-    free(const_cast<char*>(sName));
-    sName = 0;
 }
 
 void PythonGroupCommand::activated(int iMsg)