Re: [Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
Hi Ian, On 05/05/2019 15:58, Ian McInerney wrote: I saw that on the todo list along with the auto_ptr replacement and decided to take a stab at it. Thank you for taking the initiative here, and thanks also for updating the TODO list! doing the auto_ptr replacement part since when I looked through the code it seems they are only used in the sch_lib_table files, which I believe would be part of the overhaul for v6 with the new file formats. Whoever ends up working with those parts can do the replacements then Yep, it's likely to be an area of heavy development. C++17 is (sadly) not likely to be in KiCad any time soon, as the older supported platforms don't have compiler support by default. that will take care of all the parts in the C++11 technical todo list. There's probably more, if you see anything deserving of a place, let me know, and I'll add it to the list. Thanks again, John ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
Am 2019-05-05 18:18, schrieb Wayne Stambaugh: Seth, On 5/3/19 8:20 PM, Seth Hillbrand wrote: This looks good Ian. I had been meaning to get around to this but you beat me to it! Nice work. Thank you for your contribution! I don't think we need to push this to 5.1 but it's nice to have in master. -Seth We might want to consider cherry-picking this into 5.1 just to keep the code bases as close as possible. This may help prevent some conflicts down the road. Fair point. I'll give this a couple weeks to surface any issues and then push it down. -Seth ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
Seth, On 5/3/19 8:20 PM, Seth Hillbrand wrote: > Am 2019-05-03 14:04, schrieb Ian McInerney: >> This patch series removes the last uses of MUTEX and MUTLOCK from the >> code base, so now everything is using std::mutex and std::lock_guard >> instead of the Boost provided classes. I have also removed the >> ki_mutex.h header file since this is no longer needed (and found an >> extraneous inclusion in the process). >> >> This was made against the master branch. It appears the kicad_curl.cpp >> file has diverged due to openssl changes between 5.1 and master, which >> makes this not apply cleanly to 5.1. > > > This looks good Ian. I had been meaning to get around to this but you > beat me to it! Nice work. Thank you for your contribution! > > I don't think we need to push this to 5.1 but it's nice to have in master. > > -Seth We might want to consider cherry-picking this into 5.1 just to keep the code bases as close as possible. This may help prevent some conflicts down the road. Wayne ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
Seth, Thanks for pushing it. I saw that on the todo list along with the auto_ptr replacement and decided to take a stab at it. I didn't end up doing the auto_ptr replacement part since when I looked through the code it seems they are only used in the sch_lib_table files, which I believe would be part of the overhaul for v6 with the new file formats. Whoever ends up working with those parts can do the replacements then and that will take care of all the parts in the C++11 technical todo list. -Ian On Sat, May 4, 2019 at 1:20 AM Seth Hillbrand wrote: > Am 2019-05-03 14:04, schrieb Ian McInerney: > > This patch series removes the last uses of MUTEX and MUTLOCK from the > > code base, so now everything is using std::mutex and std::lock_guard > > instead of the Boost provided classes. I have also removed the > > ki_mutex.h header file since this is no longer needed (and found an > > extraneous inclusion in the process). > > > > This was made against the master branch. It appears the kicad_curl.cpp > > file has diverged due to openssl changes between 5.1 and master, which > > makes this not apply cleanly to 5.1. > > > This looks good Ian. I had been meaning to get around to this but you > beat me to it! Nice work. Thank you for your contribution! > > I don't think we need to push this to 5.1 but it's nice to have in > master. > > -Seth > ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
Re: [Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
Am 2019-05-03 14:04, schrieb Ian McInerney: This patch series removes the last uses of MUTEX and MUTLOCK from the code base, so now everything is using std::mutex and std::lock_guard instead of the Boost provided classes. I have also removed the ki_mutex.h header file since this is no longer needed (and found an extraneous inclusion in the process). This was made against the master branch. It appears the kicad_curl.cpp file has diverged due to openssl changes between 5.1 and master, which makes this not apply cleanly to 5.1. This looks good Ian. I had been meaning to get around to this but you beat me to it! Nice work. Thank you for your contribution! I don't think we need to push this to 5.1 but it's nice to have in master. -Seth ___ Mailing list: https://launchpad.net/~kicad-developers Post to : kicad-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~kicad-developers More help : https://help.launchpad.net/ListHelp
[Kicad-developers] [PATCH] Replace remaining Boost Mutexes with std::mutex
This patch series removes the last uses of MUTEX and MUTLOCK from the code base, so now everything is using std::mutex and std::lock_guard instead of the Boost provided classes. I have also removed the ki_mutex.h header file since this is no longer needed (and found an extraneous inclusion in the process). This was made against the master branch. It appears the kicad_curl.cpp file has diverged due to openssl changes between 5.1 and master, which makes this not apply cleanly to 5.1. Thanks, -Ian From 34e1977ef038d55baf03e73eda772f79d2f4fe9e Mon Sep 17 00:00:00 2001 From: Ian McInerney Date: Fri, 3 May 2019 18:28:11 +0100 Subject: [PATCH 1/2] Replace remaining Boost mutexs with std::mutex CHANGED: Replaced all MUTEX types with std::mutex Replaced all MUTLOCK types with std::lock_guard --- common/common.cpp | 6 +++--- common/kicad_curl/kicad_curl.cpp | 18 +- include/gl_context_mgr.h | 4 ++-- pcbnew/footprint_preview_panel.cpp | 24 4 files changed, 26 insertions(+), 26 deletions(-) diff --git a/common/common.cpp b/common/common.cpp index 8f27b8032..0316f28dd 100644 --- a/common/common.cpp +++ b/common/common.cpp @@ -34,6 +34,7 @@ #include #include #include +#include #include #include @@ -454,14 +455,13 @@ wxString KIwxExpandEnvVars(const wxString& str) } -#include const wxString ExpandEnvVarSubstitutions( const wxString& aString ) { // wxGetenv( wchar_t* ) is not re-entrant on linux. // Put a lock on multithreaded use of wxGetenv( wchar_t* ), called from wxEpandEnvVars(), -static MUTEXgetenv_mutex; +static std::mutex getenv_mutex; -MUTLOCK lock( getenv_mutex ); +std::lock_guard lock( getenv_mutex ); // We reserve the right to do this another way, by providing our own member function. return KIwxExpandEnvVars( aString ); diff --git a/common/kicad_curl/kicad_curl.cpp b/common/kicad_curl/kicad_curl.cpp index ea32691b2..2290cb763 100644 --- a/common/kicad_curl/kicad_curl.cpp +++ b/common/kicad_curl/kicad_curl.cpp @@ -27,7 +27,7 @@ // conflicts for some defines, at least on Windows #include -#include// MUTEX and MUTLOCK +#include #include// THROW_IO_ERROR @@ -37,14 +37,14 @@ // client (API) header file. static volatile bool s_initialized; -static MUTEX s_lock;// for s_initialized +static std::mutex s_lock;// for s_initialized // Assume that on these platforms libcurl uses OpenSSL #if defined(__linux__) || defined(__MINGW32__) #include -static MUTEX* s_crypto_locks; +static std::mutex* s_crypto_locks; /* * From OpenSSL v1.1.0, the CRYPTO_set_locking_callback macro is a no-op. @@ -83,7 +83,7 @@ static void lock_callback( int mode, int type, const char* file, int line ) static void init_locks() { -s_crypto_locks = new MUTEX[ CRYPTO_num_locks() ]; +s_crypto_locks = new std::mutex[ CRYPTO_num_locks() ]; // From http://linux.die.net/man/3/crypto_set_id_callback: @@ -154,7 +154,7 @@ void KICAD_CURL::Init() // will not need to lock. if( !s_initialized ) { -MUTLOCK lock( s_lock ); +std::lock_guard lock( s_lock ); if( !s_initialized ) { @@ -177,22 +177,22 @@ void KICAD_CURL::Cleanup() { /* -Calling MUTLOCK() from a static destructor will typically be bad, since the +Calling lock_guard() from a static destructor will typically be bad, since the s_lock may already have been statically destroyed itself leading to a boost exception. (Remember C++ does not provide certain sequencing of static destructor invocation.) -To prevent this we test s_initialized twice, which ensures that the MUTLOCK +To prevent this we test s_initialized twice, which ensures that the lock_guard is only instantiated on the first call, which should be from PGM_BASE::destroy() which is first called earlier than static destruction. Then when called again from the actual PGM_BASE::~PGM_BASE() function, -MUTLOCK will not be instantiated because s_initialized will be false. +lock_guard will not be instantiated because s_initialized will be false. */ if( s_initialized ) { -MUTLOCK lock( s_lock ); +std::lock_guard lock( s_lock ); if( s_initialized ) { diff --git a/include/gl_context_mgr.h b/include/gl_context_mgr.h index 1d74fefc2..b5812d263 100644 --- a/include/gl_context_mgr.h +++ b/include/gl_context_mgr.h @@ -26,7 +26,7 @@ #define GL_CONTEXT_MANAGER_H #include -#include +#include #include class GL_CONTEXT_MANAGER @@ -89,7 +89,7 @@ private: wxGLContext* m_glCtx; ///> Lock to prevent unexpected GL context switching. -MUTEX m_glCtxMutex; +std::mutex m_glCtxMutex; // Singleton GL_CONTEXT_MANAGER(); diff --git a/pcbnew/footprint_preview_panel.cpp b/pcbnew/footprint_preview_panel.cpp index 5052e81bc..ad3416ff5 100644 ---