From e6945aeedc05f1e71e43a8da012ab3e5b0606ca9 Mon Sep 17 00:00:00 2001 From: Lauri Nurmi Date: Fri, 29 Nov 2019 22:54:20 +0200 Subject: [PATCH] Eliminate public header file's dependency on CoreServices.h wx public headers are not supposed to include platform-specific headers defining many macros that can conflict with the identifiers defined in the application code. In this particular case, including CoreServices.h ultimately #included AssertMacros.h, which by default on older SDKs (<10.12) introduces various macros whose names very easily conflict with user code. For example, if you #included in your own code, and your code happened to contain a symbol called 'check', or 'verify', compilation failed. Fix this by using pImpl idiom to move the variable requiring a type defined in the SDK header into the source file. Closes https://github.com/wxWidgets/wxWidgets/pull/1666 --- include/wx/osx/fswatcher_fsevents.h | 9 ++++--- src/osx/fswatcher_fsevents.cpp | 37 ++++++++++++++++++----------- 2 files changed, 27 insertions(+), 19 deletions(-) diff --git a/include/wx/osx/fswatcher_fsevents.h b/include/wx/osx/fswatcher_fsevents.h index bbaa95d951..3a3843c472 100644 --- a/include/wx/osx/fswatcher_fsevents.h +++ b/include/wx/osx/fswatcher_fsevents.h @@ -15,11 +15,8 @@ #if wxUSE_FSWATCHER -#include #include "wx/unix/fswatcher_kqueue.h" -WX_DECLARE_STRING_HASH_MAP(FSEventStreamRef, FSEventStreamRefMap); - /* The FSEvents watcher uses the newer FSEvents service that is available in OS X, the service allows for @@ -78,8 +75,10 @@ public: private: - // map of path => FSEventStreamRef - FSEventStreamRefMap m_streams; + // use the pImpl idiom to eliminate this header's dependency + // on CoreServices.h (and ultimately AssertMacros.h) + struct PrivateData; + PrivateData *m_pImpl; }; diff --git a/src/osx/fswatcher_fsevents.cpp b/src/osx/fswatcher_fsevents.cpp index 2d5573c256..37806db510 100644 --- a/src/osx/fswatcher_fsevents.cpp +++ b/src/osx/fswatcher_fsevents.cpp @@ -22,6 +22,7 @@ #include "wx/osx/core/private/strconv_cf.h" #include +#include // A small class that we will give the FSEvents // framework, which will be forwarded to the function @@ -273,22 +274,30 @@ static void wxDeleteContext(const void* context) delete watcherContext; } +WX_DECLARE_STRING_HASH_MAP(FSEventStreamRef, FSEventStreamRefMap); + +struct wxFsEventsFileSystemWatcher::PrivateData +{ + // map of path => FSEventStreamRef + FSEventStreamRefMap m_streams; +}; + wxFsEventsFileSystemWatcher::wxFsEventsFileSystemWatcher() -: wxKqueueFileSystemWatcher() +: wxKqueueFileSystemWatcher(), m_pImpl(new PrivateData) { } wxFsEventsFileSystemWatcher::wxFsEventsFileSystemWatcher(const wxFileName& path, int events) -: wxKqueueFileSystemWatcher(path, events) +: wxKqueueFileSystemWatcher(path, events), m_pImpl(new PrivateData) { } wxFsEventsFileSystemWatcher::~wxFsEventsFileSystemWatcher() { - + delete m_pImpl; } bool wxFsEventsFileSystemWatcher::AddTree(const wxFileName& path, int events, @@ -313,7 +322,7 @@ bool wxFsEventsFileSystemWatcher::AddTree(const wxFileName& path, int events, return false; } - if ( m_streams.find(canonical) != m_streams.end() ) + if ( m_pImpl->m_streams.find(canonical) != m_pImpl->m_streams.end() ) { // How to take into account filespec // if client adds a watch for /home/*.cpp @@ -365,7 +374,7 @@ bool wxFsEventsFileSystemWatcher::AddTree(const wxFileName& path, int events, started = FSEventStreamStart(stream); if ( started ) { - m_streams[canonical] = stream; + m_pImpl->m_streams[canonical] = stream; } } @@ -398,14 +407,14 @@ bool wxFsEventsFileSystemWatcher::RemoveTree(const wxFileName& path) } } - FSEventStreamRefMap::iterator it = m_streams.find(canonical); + FSEventStreamRefMap::iterator it = m_pImpl->m_streams.find(canonical); bool removed = false; - if ( it != m_streams.end() ) + if ( it != m_pImpl->m_streams.end() ) { FSEventStreamStop(it->second); FSEventStreamInvalidate(it->second); FSEventStreamRelease(it->second); - m_streams.erase(it); + m_pImpl->m_streams.erase(it); removed = true; } return removed; @@ -415,8 +424,8 @@ bool wxFsEventsFileSystemWatcher::RemoveAll() { // remove all watches created with Add() bool ret = wxKqueueFileSystemWatcher::RemoveAll(); - FSEventStreamRefMap::iterator it = m_streams.begin(); - while ( it != m_streams.end() ) + FSEventStreamRefMap::iterator it = m_pImpl->m_streams.begin(); + while ( it != m_pImpl->m_streams.end() ) { FSEventStreamStop(it->second); FSEventStreamInvalidate(it->second); @@ -424,7 +433,7 @@ bool wxFsEventsFileSystemWatcher::RemoveAll() ++it; ret = true; } - m_streams.clear(); + m_pImpl->m_streams.clear(); return ret; } @@ -489,7 +498,7 @@ void wxFsEventsFileSystemWatcher::PostError(const wxString& msg) int wxFsEventsFileSystemWatcher::GetWatchedPathsCount() const { - return m_streams.size() + wxFileSystemWatcherBase::GetWatchedPathsCount(); + return m_pImpl->m_streams.size() + wxFileSystemWatcherBase::GetWatchedPathsCount(); } int wxFsEventsFileSystemWatcher::GetWatchedPaths(wxArrayString* paths) const @@ -500,8 +509,8 @@ int wxFsEventsFileSystemWatcher::GetWatchedPaths(wxArrayString* paths) const return 0; } wxFileSystemWatcherBase::GetWatchedPaths(paths); - FSEventStreamRefMap::const_iterator it = m_streams.begin(); - for ( ; it != m_streams.end(); ++it ) + FSEventStreamRefMap::const_iterator it = m_pImpl->m_streams.begin(); + for ( ; it != m_pImpl->m_streams.end(); ++it ) { paths->push_back(it->first); }