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

@@ -40,19 +40,19 @@ class FilesystemNodeFactory
     enum Type { SYSTEM, ZIP };
 
   public:
-    static AbstractFSNode* create(const string& path, Type type)
+    static unique_ptr<AbstractFSNode> 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<FilesystemNodePOSIX>(path);
       #elif defined(BSPF_WINDOWS)
-          return new FilesystemNodeWINDOWS(path);
+          return make_unique<FilesystemNodeWINDOWS>(path);
       #endif
           break;
         case ZIP:
-          return new FilesystemNodeZIP(path);
+          return make_unique<FilesystemNodeZIP>(path);
           break;
       }
       return nullptr;

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<AbstractFSNode>(nullptr);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -92,9 +90,7 @@ FilesystemNodeZIP::FilesystemNodeZIP(const string& p)
   else
     _isDirectory = true;
 
-  AbstractFSNode* tmp =
+  _realNode = FilesystemNodeFactory::create(_zipFile, FilesystemNodeFactory::SYSTEM);
-    FilesystemNodeFactory::create(_zipFile, FilesystemNodeFactory::SYSTEM);
-  _realNode = shared_ptr<AbstractFSNode>(tmp);
 
   setFlags(_zipFile, _virtualPath, _realNode);
 }
@@ -102,7 +98,7 @@ FilesystemNodeZIP::FilesystemNodeZIP(const string& p)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 FilesystemNodeZIP::FilesystemNodeZIP(
     const string& zipfile, const string& virtualpath,
-    shared_ptr<AbstractFSNode> 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<AbstractFSNode> 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<FilesystemNodeZIP>(string(start, end - start - 1));
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -

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<AbstractFSNode> realnode, bool isdir);
+        AbstractFSNodePtr realnode, bool isdir);
 
     void setFlags(const string& zipfile, const string& virtualpath,
-        shared_ptr<AbstractFSNode> realnode);
+        AbstractFSNodePtr realnode);
 
     friend ostream& operator<<(ostream& os, const FilesystemNodeZIP& node)
     {
@@ -91,7 +91,7 @@ class FilesystemNodeZIP : public AbstractFSNode
       ZIPERR_NO_ROMS
     };
 
-    shared_ptr<AbstractFSNode> _realNode;
+    AbstractFSNodePtr _realNode;
     string _zipFile, _virtualPath;
     string _name, _path, _shortPath;
     zip_error _error;

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 = FilesystemNodeFactory::create(p, FilesystemNodeFactory::SYSTEM);
-
-  _realNode = shared_ptr<AbstractFSNode>(tmp);
 }
 
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
@@ -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;
 }
 

src/emucore/FSNode.hxx

@@ -49,6 +49,7 @@
 
 class FilesystemNode;
 class AbstractFSNode;
+using AbstractFSNodePtr = shared_ptr<AbstractFSNode>;
 
 /**
  * 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<AbstractFSNode> _realNode;
+    AbstractFSNodePtr _realNode;
-    FilesystemNode(AbstractFSNode* realNode);
+    FilesystemNode(AbstractFSNodePtr realNode);
 };
 
 
@@ -275,7 +276,7 @@ class FilesystemNode
  * the semantics.
  */
 
-using AbstractFSList = vector<AbstractFSNode*>;
+using AbstractFSList = vector<AbstractFSNodePtr>;
 
 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

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<FilesystemNodePOSIX>(string(start, size_t(end - start)));
 }

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;

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<FilesystemNodeWINDOWS>(string(start, size_t(end - start)));
   }
   else
-    return new FilesystemNodeWINDOWS();
+    return make_shared<FilesystemNodeWINDOWS>();
 }

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;