From 628514bcd3b1a564c17f75456b0f7888ae7bed4a Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Mar 2021 20:07:42 +0100 Subject: [PATCH 1/4] Rename static wxModule::m_modules to use "ms_" prefix No real changes, just use consisting naming convention. Better late than never. --- include/wx/module.h | 6 +++--- src/common/module.cpp | 14 +++++++------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/wx/module.h b/include/wx/module.h index 993f765e0f..f139ddde12 100644 --- a/include/wx/module.h +++ b/include/wx/module.h @@ -54,14 +54,14 @@ public: static void RegisterModule(wxModule *module); static void RegisterModules(); static bool InitializeModules(); - static void CleanUpModules() { DoCleanUpModules(m_modules); } + static void CleanUpModules() { DoCleanUpModules(ms_modules); } // used by wxObjectLoader when unloading shared libs's static void UnregisterModule(wxModule *module); protected: - static wxModuleList m_modules; + static wxModuleList ms_modules; // the function to call from constructor of a deriving class add module // dependency which will be initialized before the module and unloaded @@ -89,7 +89,7 @@ private: // cleanup the modules in the specified list (which may not contain all // modules if we're called during initialization because not all modules - // could be initialized) and also empty m_modules itself + // could be initialized) and also empty ms_modules itself static void DoCleanUpModules(const wxModuleList& modules); // resolve all named dependencies and add them to the normal m_dependencies diff --git a/src/common/module.cpp b/src/common/module.cpp index 994f3d8093..45a05d35ff 100644 --- a/src/common/module.cpp +++ b/src/common/module.cpp @@ -28,17 +28,17 @@ WX_DEFINE_LIST(wxModuleList) wxIMPLEMENT_ABSTRACT_CLASS(wxModule, wxObject) -wxModuleList wxModule::m_modules; +wxModuleList wxModule::ms_modules; void wxModule::RegisterModule(wxModule* module) { module->m_state = State_Registered; - m_modules.Append(module); + ms_modules.Append(module); } void wxModule::UnregisterModule(wxModule* module) { - m_modules.DeleteObject(module); + ms_modules.DeleteObject(module); delete module; } @@ -101,7 +101,7 @@ bool wxModule::DoInitializeModule(wxModule *module, } // find the module in the registered modules list - for ( node = m_modules.GetFirst(); node; node = node->GetNext() ) + for ( node = ms_modules.GetFirst(); node; node = node->GetNext() ) { wxModule *moduleDep = node->GetData(); if ( moduleDep->GetClassInfo() == cinfo ) @@ -146,7 +146,7 @@ bool wxModule::InitializeModules() { wxModuleList initializedModules; - for ( wxModuleList::compatibility_iterator node = m_modules.GetFirst(); + for ( wxModuleList::compatibility_iterator node = ms_modules.GetFirst(); node; node = node->GetNext() ) { @@ -168,7 +168,7 @@ bool wxModule::InitializeModules() } // remember the real initialisation order - m_modules = initializedModules; + ms_modules = initializedModules; return true; } @@ -195,7 +195,7 @@ void wxModule::DoCleanUpModules(const wxModuleList& modules) } // clear all modules, even the non-initialized ones - WX_CLEAR_LIST(wxModuleList, m_modules); + WX_CLEAR_LIST(wxModuleList, ms_modules); } bool wxModule::ResolveNamedDependencies() From e10e72112071b0e85e9c58ea96fb38e95ed9aa91 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Mar 2021 20:25:49 +0100 Subject: [PATCH 2/4] Get rid of wxList and wxArray in wxModule code Simply use wxVector instead, this shouldn't be less efficient (we rarely remove the modules from the list and iterating over a vector should actually be faster, as well as consuming less memory), but it avoids ugly macros, is simpler to use and to debug and will be trivial to replace with std::vector<> in the future. No real changes, this is just pure cleanup. --- include/wx/module.h | 20 +++++-------- src/common/module.cpp | 65 ++++++++++++++++++++++++++----------------- 2 files changed, 47 insertions(+), 38 deletions(-) diff --git a/include/wx/module.h b/include/wx/module.h index f139ddde12..41ef501e7e 100644 --- a/include/wx/module.h +++ b/include/wx/module.h @@ -12,18 +12,11 @@ #define _WX_MODULE_H_ #include "wx/object.h" -#include "wx/list.h" -#include "wx/arrstr.h" -#include "wx/dynarray.h" +#include "wx/vector.h" -// declare a linked list of modules -class WXDLLIMPEXP_FWD_BASE wxModule; -WX_DECLARE_USER_EXPORTED_LIST(wxModule, wxModuleList, WXDLLIMPEXP_BASE); - -// and an array of class info objects -WX_DEFINE_USER_EXPORTED_ARRAY_PTR(wxClassInfo *, wxArrayClassInfo, - class WXDLLIMPEXP_BASE); +class wxModule; +typedef wxVector wxModuleList; // declaring a class derived from wxModule will automatically create an // instance of this class on program startup, call its OnInit() method and call @@ -70,14 +63,14 @@ protected: { wxCHECK_RET( dep, wxT("NULL module dependency") ); - m_dependencies.Add(dep); + m_dependencies.push_back(dep); } // same as the version above except it will look up wxClassInfo by name on // its own. Note that className must be ASCII void AddDependency(const char *className) { - m_namedDependencies.Add(wxASCII_STR(className)); + m_namedDependencies.push_back(wxASCII_STR(className)); } @@ -98,11 +91,12 @@ private: // module dependencies: contains wxClassInfo pointers for all modules which // must be initialized before this one + typedef wxVector wxArrayClassInfo; wxArrayClassInfo m_dependencies; // and the named dependencies: those will be resolved during run-time and // added to m_dependencies - wxArrayString m_namedDependencies; + wxVector m_namedDependencies; // used internally while initializing/cleaning up modules enum diff --git a/src/common/module.cpp b/src/common/module.cpp index 45a05d35ff..f72956cf6d 100644 --- a/src/common/module.cpp +++ b/src/common/module.cpp @@ -20,12 +20,8 @@ #include "wx/log.h" #endif -#include "wx/listimpl.cpp" - #define TRACE_MODULE wxT("module") -WX_DEFINE_LIST(wxModuleList) - wxIMPLEMENT_ABSTRACT_CLASS(wxModule, wxObject) wxModuleList wxModule::ms_modules; @@ -33,12 +29,22 @@ wxModuleList wxModule::ms_modules; void wxModule::RegisterModule(wxModule* module) { module->m_state = State_Registered; - ms_modules.Append(module); + ms_modules.push_back(module); } void wxModule::UnregisterModule(wxModule* module) { - ms_modules.DeleteObject(module); + for ( wxModuleList::iterator it = ms_modules.begin(); + it != ms_modules.end(); + ++it ) + { + if ( *it == module ) + { + ms_modules.erase(it); + break; + } + } + delete module; } @@ -87,23 +93,25 @@ bool wxModule::DoInitializeModule(wxModule *module, wxClassInfo * cinfo = dependencies[i]; // Check if the module is already initialized - wxModuleList::compatibility_iterator node; - for ( node = initializedModules.GetFirst(); node; node = node->GetNext() ) + wxModuleList::const_iterator it; + for ( it = initializedModules.begin(); + it != initializedModules.end(); + ++it ) { - if ( node->GetData()->GetClassInfo() == cinfo ) + if ( (*it)->GetClassInfo() == cinfo ) break; } - if ( node ) + if ( it != initializedModules.end() ) { // this dependency is already initialized, nothing to do continue; } // find the module in the registered modules list - for ( node = ms_modules.GetFirst(); node; node = node->GetNext() ) + for ( it = ms_modules.begin(); it != ms_modules.end(); ++it ) { - wxModule *moduleDep = node->GetData(); + wxModule *moduleDep = *it; if ( moduleDep->GetClassInfo() == cinfo ) { if ( !DoInitializeModule(moduleDep, initializedModules ) ) @@ -116,7 +124,7 @@ bool wxModule::DoInitializeModule(wxModule *module, } } - if ( !node ) + if ( it == ms_modules.end() ) { wxLogError(_("Dependency \"%s\" of module \"%s\" doesn't exist."), cinfo->GetClassName(), @@ -136,7 +144,7 @@ bool wxModule::DoInitializeModule(wxModule *module, module->GetClassInfo()->GetClassName()); module->m_state = State_Initialized; - initializedModules.Append(module); + initializedModules.push_back(module); return true; } @@ -146,11 +154,11 @@ bool wxModule::InitializeModules() { wxModuleList initializedModules; - for ( wxModuleList::compatibility_iterator node = ms_modules.GetFirst(); - node; - node = node->GetNext() ) + for ( wxModuleList::const_iterator it = ms_modules.begin(); + it != ms_modules.end(); + ++it ) { - wxModule *module = node->GetData(); + wxModule *module = *it; // the module could have been already initialized as dependency of // another one @@ -178,14 +186,14 @@ void wxModule::DoCleanUpModules(const wxModuleList& modules) { // cleanup user-defined modules in the reverse order compared to their // initialization -- this ensures that dependencies are respected - for ( wxModuleList::compatibility_iterator node = modules.GetLast(); - node; - node = node->GetPrevious() ) + for ( wxModuleList::const_reverse_iterator rit = modules.rbegin(); + rit != modules.rend(); + ++rit ) { wxLogTrace(TRACE_MODULE, wxT("Cleanup module %s"), - node->GetData()->GetClassInfo()->GetClassName()); + (*rit)->GetClassInfo()->GetClassName()); - wxModule * module = node->GetData(); + wxModule * module = *rit; wxASSERT_MSG( module->m_state == State_Initialized, wxT("not initialized module being cleaned up") ); @@ -195,7 +203,14 @@ void wxModule::DoCleanUpModules(const wxModuleList& modules) } // clear all modules, even the non-initialized ones - WX_CLEAR_LIST(wxModuleList, ms_modules); + for ( wxModuleList::const_iterator it = ms_modules.begin(); + it != ms_modules.end(); + ++it ) + { + delete *it; + } + + ms_modules.clear(); } bool wxModule::ResolveNamedDependencies() @@ -214,7 +229,7 @@ bool wxModule::ResolveNamedDependencies() // add it even if it is not derived from wxModule because // DoInitializeModule() will make sure a module with the same class // info exists and fail if it doesn't - m_dependencies.Add(info); + m_dependencies.push_back(info); } return true; From fbd23270e3d0552b223ffb6f8b509092d7c11520 Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Mar 2021 20:32:26 +0100 Subject: [PATCH 3/4] Remove CppUnit boilerplate from wxModule unit test Also use wxString instead of fixed size char array and wxStrcat(). --- tests/misc/module.cpp | 31 +++++-------------------------- 1 file changed, 5 insertions(+), 26 deletions(-) diff --git a/tests/misc/module.cpp b/tests/misc/module.cpp index efb8d64cf7..d05b85ec82 100644 --- a/tests/misc/module.cpp +++ b/tests/misc/module.cpp @@ -19,12 +19,12 @@ // test classes derived from wxModule // ---------------------------------------------------------------------------- -char g_strLoadOrder[256] = "\0"; +wxString g_strLoadOrder; class Module : public wxModule { protected: - virtual bool OnInit() wxOVERRIDE { wxStrcat(g_strLoadOrder, GetClassInfo()->GetClassName()); return true; } + virtual bool OnInit() wxOVERRIDE { g_strLoadOrder += GetClassInfo()->GetClassName(); return true; } virtual void OnExit() wxOVERRIDE { } }; @@ -86,32 +86,11 @@ ModuleD::ModuleD() } // ---------------------------------------------------------------------------- -// test class +// tests themselves // ---------------------------------------------------------------------------- -class ModuleTestCase : public CppUnit::TestCase -{ -public: - ModuleTestCase() { } - -private: - CPPUNIT_TEST_SUITE( ModuleTestCase ); - CPPUNIT_TEST( LoadOrder ); - CPPUNIT_TEST_SUITE_END(); - - void LoadOrder(); - wxDECLARE_NO_COPY_CLASS(ModuleTestCase); -}; - -// register in the unnamed registry so that these tests are run by default -CPPUNIT_TEST_SUITE_REGISTRATION( ModuleTestCase ); - -// also include in its own registry so that these tests can be run alone -CPPUNIT_TEST_SUITE_NAMED_REGISTRATION( ModuleTestCase, "ModuleTestCase" ); - -void ModuleTestCase::LoadOrder() +TEST_CASE("wxModule::LoadOrder", "[module]") { // module D is the only one with no dependencies and so should load as first (and so on): - CPPUNIT_ASSERT_EQUAL( std::string("ModuleDModuleCModuleBModuleA"), - g_strLoadOrder ); + CHECK( g_strLoadOrder == "ModuleDModuleCModuleBModuleA" ); } From 5ddf57c150d2289701303195d089b4c1cafad87f Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Sun, 7 Mar 2021 20:37:09 +0100 Subject: [PATCH 4/4] Add wxModule::AreInitialized() This internal function will be useful to check if the modules are already initialized, i.e. if the library is in the "steady state" between the end of the initialization and the beginning of the cleanup phases. --- include/wx/module.h | 5 ++++- src/common/module.cpp | 10 ++++++++++ tests/misc/module.cpp | 8 ++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/include/wx/module.h b/include/wx/module.h index 41ef501e7e..1f107029aa 100644 --- a/include/wx/module.h +++ b/include/wx/module.h @@ -47,7 +47,8 @@ public: static void RegisterModule(wxModule *module); static void RegisterModules(); static bool InitializeModules(); - static void CleanUpModules() { DoCleanUpModules(ms_modules); } + static void CleanUpModules(); + static bool AreInitialized() { return ms_areInitialized; } // used by wxObjectLoader when unloading shared libs's @@ -56,6 +57,8 @@ public: protected: static wxModuleList ms_modules; + static bool ms_areInitialized; + // the function to call from constructor of a deriving class add module // dependency which will be initialized before the module and unloaded // after that diff --git a/src/common/module.cpp b/src/common/module.cpp index f72956cf6d..7344cfbd96 100644 --- a/src/common/module.cpp +++ b/src/common/module.cpp @@ -25,6 +25,7 @@ wxIMPLEMENT_ABSTRACT_CLASS(wxModule, wxObject) wxModuleList wxModule::ms_modules; +bool wxModule::ms_areInitialized = false; void wxModule::RegisterModule(wxModule* module) { @@ -178,9 +179,18 @@ bool wxModule::InitializeModules() // remember the real initialisation order ms_modules = initializedModules; + ms_areInitialized = true; + return true; } +void wxModule::CleanUpModules() +{ + DoCleanUpModules(ms_modules); + + ms_areInitialized = false; +} + // Clean up all currently initialized modules void wxModule::DoCleanUpModules(const wxModuleList& modules) { diff --git a/tests/misc/module.cpp b/tests/misc/module.cpp index d05b85ec82..24ee867e28 100644 --- a/tests/misc/module.cpp +++ b/tests/misc/module.cpp @@ -15,6 +15,8 @@ #include "wx/module.h" #include "wx/wxcrt.h" // for wxStrcat() +static bool gs_wasInitialized = wxModule::AreInitialized(); + // ---------------------------------------------------------------------------- // test classes derived from wxModule // ---------------------------------------------------------------------------- @@ -89,6 +91,12 @@ ModuleD::ModuleD() // tests themselves // ---------------------------------------------------------------------------- +TEST_CASE("wxModule::Initialized", "[module]") +{ + CHECK( !gs_wasInitialized ); + CHECK( wxModule::AreInitialized() ); +} + TEST_CASE("wxModule::LoadOrder", "[module]") { // module D is the only one with no dependencies and so should load as first (and so on):