From f2f952f09b8fe1909b0e446b47b3bb2ed1e67760 Mon Sep 17 00:00:00 2001 From: Stephen Anthony Date: Sun, 5 Aug 2018 22:32:11 -0230 Subject: [PATCH] Reworking of the FSNode classes, using smart pointers. - For now, I simply use shared_ptr; long term, I will look into unique_ptr for more efficiency. --- src/common/FSNodeFactory.hxx | 8 ++++---- src/common/FSNodeZIP.cxx | 14 +++++--------- src/common/FSNodeZIP.hxx | 8 ++++---- src/emucore/FSNode.cxx | 12 ++++-------- src/emucore/FSNode.hxx | 11 ++++++----- src/unix/FSNodePOSIX.cxx | 12 ++++++------ src/unix/FSNodePOSIX.hxx | 2 +- src/windows/FSNodeWINDOWS.cxx | 6 +++--- src/windows/FSNodeWINDOWS.hxx | 2 +- 9 files changed, 34 insertions(+), 41 deletions(-) diff --git a/src/common/FSNodeFactory.hxx b/src/common/FSNodeFactory.hxx index c3b958c2e..4519c95d5 100644 --- a/src/common/FSNodeFactory.hxx +++ b/src/common/FSNodeFactory.hxx @@ -40,19 +40,19 @@ class FilesystemNodeFactory enum Type { SYSTEM, ZIP }; public: - static AbstractFSNode* create(const string& path, Type type) + static unique_ptr create(const string& path, Type type) { switch(type) { case SYSTEM: #if defined(BSPF_UNIX) || defined(BSPF_MAC_OSX) - return new FilesystemNodePOSIX(path); + return make_unique(path); #elif defined(BSPF_WINDOWS) - return new FilesystemNodeWINDOWS(path); + return make_unique(path); #endif break; case ZIP: - return new FilesystemNodeZIP(path); + return make_unique(path); break; } return nullptr; diff --git a/src/common/FSNodeZIP.cxx b/src/common/FSNodeZIP.cxx index 0528e9469..0af148608 100644 --- a/src/common/FSNodeZIP.cxx +++ b/src/common/FSNodeZIP.cxx @@ -29,8 +29,6 @@ FilesystemNodeZIP::FilesystemNodeZIP() _isDirectory(false), _isFile(false) { - // We need a name, else the node is invalid - _realNode = shared_ptr(nullptr); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -92,9 +90,7 @@ FilesystemNodeZIP::FilesystemNodeZIP(const string& p) else _isDirectory = true; - AbstractFSNode* tmp = - FilesystemNodeFactory::create(_zipFile, FilesystemNodeFactory::SYSTEM); - _realNode = shared_ptr(tmp); + _realNode = FilesystemNodeFactory::create(_zipFile, FilesystemNodeFactory::SYSTEM); setFlags(_zipFile, _virtualPath, _realNode); } @@ -102,7 +98,7 @@ FilesystemNodeZIP::FilesystemNodeZIP(const string& p) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - FilesystemNodeZIP::FilesystemNodeZIP( const string& zipfile, const string& virtualpath, - shared_ptr realnode, bool isdir) + AbstractFSNodePtr realnode, bool isdir) : _error(ZIPERR_NONE), _numFiles(0), _isDirectory(isdir), @@ -114,7 +110,7 @@ FilesystemNodeZIP::FilesystemNodeZIP( // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - void FilesystemNodeZIP::setFlags(const string& zipfile, const string& virtualpath, - shared_ptr realnode) + AbstractFSNodePtr realnode) { _zipFile = zipfile; _virtualPath = virtualpath; @@ -195,7 +191,7 @@ uInt32 FilesystemNodeZIP::read(BytePtr& image) const } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -AbstractFSNode* FilesystemNodeZIP::getParent() const +AbstractFSNodePtr FilesystemNodeZIP::getParent() const { if(_virtualPath == "") return _realNode ? _realNode->getParent() : nullptr; @@ -203,7 +199,7 @@ AbstractFSNode* FilesystemNodeZIP::getParent() const const char* start = _path.c_str(); const char* end = lastPathComponent(_path); - return new FilesystemNodeZIP(string(start, end - start - 1)); + return make_shared(string(start, end - start - 1)); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - diff --git a/src/common/FSNodeZIP.hxx b/src/common/FSNodeZIP.hxx index 29b51df00..56d83d378 100644 --- a/src/common/FSNodeZIP.hxx +++ b/src/common/FSNodeZIP.hxx @@ -61,16 +61,16 @@ class FilesystemNodeZIP : public AbstractFSNode ////////////////////////////////////////////////////////// bool getChildren(AbstractFSList& list, ListMode mode, bool hidden) const; - AbstractFSNode* getParent() const; + AbstractFSNodePtr getParent() const; uInt32 read(BytePtr& image) const; private: FilesystemNodeZIP(const string& zipfile, const string& virtualpath, - shared_ptr realnode, bool isdir); + AbstractFSNodePtr realnode, bool isdir); void setFlags(const string& zipfile, const string& virtualpath, - shared_ptr realnode); + AbstractFSNodePtr realnode); friend ostream& operator<<(ostream& os, const FilesystemNodeZIP& node) { @@ -91,7 +91,7 @@ class FilesystemNodeZIP : public AbstractFSNode ZIPERR_NO_ROMS }; - shared_ptr _realNode; + AbstractFSNodePtr _realNode; string _zipFile, _virtualPath; string _name, _path, _shortPath; zip_error _error; diff --git a/src/emucore/FSNode.cxx b/src/emucore/FSNode.cxx index c118ece2e..514131c4a 100644 --- a/src/emucore/FSNode.cxx +++ b/src/emucore/FSNode.cxx @@ -30,7 +30,7 @@ FilesystemNode::FilesystemNode() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -FilesystemNode::FilesystemNode(AbstractFSNode *realNode) +FilesystemNode::FilesystemNode(AbstractFSNodePtr realNode) : _realNode(realNode) { } @@ -38,15 +38,11 @@ FilesystemNode::FilesystemNode(AbstractFSNode *realNode) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - FilesystemNode::FilesystemNode(const string& p) { - AbstractFSNode* tmp = nullptr; - // Is this potentially a ZIP archive? if(BSPF::containsIgnoreCase(p, ".zip")) - tmp = FilesystemNodeFactory::create(p, FilesystemNodeFactory::ZIP); + _realNode = FilesystemNodeFactory::create(p, FilesystemNodeFactory::ZIP); else - tmp = FilesystemNodeFactory::create(p, FilesystemNodeFactory::SYSTEM); - - _realNode = shared_ptr(tmp); + _realNode = FilesystemNodeFactory::create(p, FilesystemNodeFactory::SYSTEM); } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - @@ -132,7 +128,7 @@ FilesystemNode FilesystemNode::getParent() const if (_realNode == nullptr) return *this; - AbstractFSNode* node = _realNode->getParent(); + AbstractFSNodePtr node = _realNode->getParent(); return node ? FilesystemNode(node) : *this; } diff --git a/src/emucore/FSNode.hxx b/src/emucore/FSNode.hxx index 4291466ce..160274a5b 100644 --- a/src/emucore/FSNode.hxx +++ b/src/emucore/FSNode.hxx @@ -49,6 +49,7 @@ class FilesystemNode; class AbstractFSNode; +using AbstractFSNodePtr = shared_ptr; /** * List of multiple file system nodes. E.g. the contents of a given directory. @@ -144,7 +145,7 @@ class FilesystemNode * @return true if successful, false otherwise (e.g. when the directory * does not exist). */ - virtual bool getChildren(FSList &fslist, ListMode mode = kListDirectoriesOnly, + virtual bool getChildren(FSList& fslist, ListMode mode = kListDirectoriesOnly, bool hidden = false) const; /** @@ -261,8 +262,8 @@ class FilesystemNode string getShortPathWithExt(const string& ext) const; // FIXME - dead code private: - shared_ptr _realNode; - FilesystemNode(AbstractFSNode* realNode); + AbstractFSNodePtr _realNode; + FilesystemNode(AbstractFSNodePtr realNode); }; @@ -275,7 +276,7 @@ class FilesystemNode * the semantics. */ -using AbstractFSList = vector; +using AbstractFSList = vector; class AbstractFSNode { @@ -403,7 +404,7 @@ class AbstractFSNode * The parent node of this directory. * The parent of the root is the root itself. */ - virtual AbstractFSNode* getParent() const = 0; + virtual AbstractFSNodePtr getParent() const = 0; }; #endif diff --git a/src/unix/FSNodePOSIX.cxx b/src/unix/FSNodePOSIX.cxx index c9ae65112..8cdff324f 100644 --- a/src/unix/FSNodePOSIX.cxx +++ b/src/unix/FSNodePOSIX.cxx @@ -97,12 +97,11 @@ bool FilesystemNodePOSIX::getChildren(AbstractFSList& myList, ListMode mode, assert(_isDirectory); DIR* dirp = opendir(_path.c_str()); - struct dirent* dp; - if (dirp == nullptr) return false; - // loop over dir entries using readdir + // Loop over dir entries using readdir + struct dirent* dp; while ((dp = readdir(dirp)) != nullptr) { // Skip 'invisible' files if necessary @@ -137,7 +136,6 @@ bool FilesystemNodePOSIX::getChildren(AbstractFSList& myList, ListMode mode, } else { - entry._isValid = (dp->d_type == DT_DIR) || (dp->d_type == DT_REG) || (dp->d_type == DT_LNK); if (dp->d_type == DT_LNK) { struct stat st; @@ -157,6 +155,8 @@ bool FilesystemNodePOSIX::getChildren(AbstractFSList& myList, ListMode mode, if (entry._isDirectory) entry._path += "/"; + + entry._isValid = entry._isDirectory || entry._isFile; } #endif @@ -226,7 +226,7 @@ bool FilesystemNodePOSIX::rename(const string& newfile) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -AbstractFSNode* FilesystemNodePOSIX::getParent() const +AbstractFSNodePtr FilesystemNodePOSIX::getParent() const { if (_path == "/") return nullptr; @@ -234,5 +234,5 @@ AbstractFSNode* FilesystemNodePOSIX::getParent() const const char* start = _path.c_str(); const char* end = lastPathComponent(_path); - return new FilesystemNodePOSIX(string(start, size_t(end - start))); + return make_unique(string(start, size_t(end - start))); } diff --git a/src/unix/FSNodePOSIX.hxx b/src/unix/FSNodePOSIX.hxx index f05720d65..d6fb7346b 100644 --- a/src/unix/FSNodePOSIX.hxx +++ b/src/unix/FSNodePOSIX.hxx @@ -72,7 +72,7 @@ class FilesystemNodePOSIX : public AbstractFSNode bool rename(const string& newfile) override; bool getChildren(AbstractFSList& list, ListMode mode, bool hidden) const override; - AbstractFSNode* getParent() const override; + AbstractFSNodePtr getParent() const override; protected: string _path; diff --git a/src/windows/FSNodeWINDOWS.cxx b/src/windows/FSNodeWINDOWS.cxx index acd948b1f..0671d1e05 100644 --- a/src/windows/FSNodeWINDOWS.cxx +++ b/src/windows/FSNodeWINDOWS.cxx @@ -296,7 +296,7 @@ bool FilesystemNodeWINDOWS::rename(const string& newfile) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -AbstractFSNode* FilesystemNodeWINDOWS::getParent() const +AbstractFSNodePtr FilesystemNodeWINDOWS::getParent() const { if(_isPseudoRoot) return nullptr; @@ -306,8 +306,8 @@ AbstractFSNode* FilesystemNodeWINDOWS::getParent() const const char* start = _path.c_str(); const char* end = lastPathComponent(_path); - return new FilesystemNodeWINDOWS(string(start, size_t(end - start))); + return make_shared(string(start, size_t(end - start))); } else - return new FilesystemNodeWINDOWS(); + return make_shared(); } diff --git a/src/windows/FSNodeWINDOWS.hxx b/src/windows/FSNodeWINDOWS.hxx index f8b601365..3a700c6b8 100644 --- a/src/windows/FSNodeWINDOWS.hxx +++ b/src/windows/FSNodeWINDOWS.hxx @@ -69,7 +69,7 @@ class FilesystemNodeWINDOWS : public AbstractFSNode bool rename(const string& newfile) override; bool getChildren(AbstractFSList& list, ListMode mode, bool hidden) const override; - AbstractFSNode* getParent() const override; + AbstractFSNodePtr getParent() const override; protected: string _displayName;