Fixed crash on loading state files; try/catch was missing for the first

bit of data loaded from the file.

Changed Serializer class to unique_ptr, eliminating d'tor.


git-svn-id: svn://svn.code.sf.net/p/stella/code/trunk@3130 8b62c5a3-ac7e-4cc8-8f21-d9a121418aba

Changes.txt

@@ -22,6 +22,9 @@
   * Fixed major bug with joysticks, where mapping was being lost on reset,
     the app would crash when plugging/unplugging certain sticks, etc.
 
+  * Fixed major (but rare) crash that could occur when state files were
+    zero'ed or corrupted.
+
   * Added dialog which shows the internal joystick database (all the
     joysticks that Stella knows about) and the ability to remove
     (currently unplugged) joysticks from this database.

src/emucore/Serializer.cxx

@@ -33,15 +33,13 @@ Serializer::Serializer(const string& filename, bool readonly)
     FilesystemNode node(filename);
     if(node.isFile() && node.isReadable())
     {
-      fstream* str = new fstream(filename.c_str(), ios::in | ios::binary);
+      unique_ptr<fstream> str = make_ptr<fstream>(filename.c_str(), ios::in | ios::binary);
       if(str && str->is_open())
       {
-        myStream = str;
+        myStream = std::move(str);
         myStream->exceptions( ios_base::failbit | ios_base::badbit | ios_base::eofbit );
         reset();
       }
-      else
-        delete str;
     }
   }
   else
@@ -56,15 +54,13 @@ Serializer::Serializer(const string& filename, bool readonly)
     fstream temp(filename.c_str(), ios::out | ios::app);
     temp.close();
 
-    fstream* str = new fstream(filename.c_str(), ios::in | ios::out | ios::binary);
+    unique_ptr<fstream> str = make_ptr<fstream>(filename.c_str(), ios::in | ios::out | ios::binary);
     if(str && str->is_open())
     {
-      myStream = str;
+      myStream = std::move(str);
       myStream->exceptions( ios_base::failbit | ios_base::badbit | ios_base::eofbit );
       reset();
     }
-    else
-      delete str;
   }
 }
 
@@ -73,7 +69,7 @@ Serializer::Serializer()
   : myStream(nullptr),
     myUseFilestream(false)
 {
-  myStream = new stringstream(ios::in | ios::out | ios::binary);
+  myStream = make_ptr<stringstream>(ios::in | ios::out | ios::binary);
   
   // For some reason, Windows and possibly OSX needs to store something in
   // the stream before it is used for the first time
@@ -85,19 +81,6 @@ Serializer::Serializer()
   }
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-Serializer::~Serializer()
-{
-  if(myStream != nullptr)
-  {
-    if(myUseFilestream)
-      ((fstream*)myStream)->close();
-
-    delete myStream;
-    myStream = nullptr;
-  }
-}
-
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool Serializer::valid() const
 {

src/emucore/Serializer.hxx

@@ -56,11 +56,6 @@ class Serializer
     Serializer(const string& filename, bool readonly = false);
     Serializer();
 
-    /**
-      Destructor
-    */
-    ~Serializer();
-
   public:
     /**
       Answers whether the serializer is currently initialized for reading
@@ -193,7 +188,7 @@ class Serializer
 
   private:
     // The stream to send the serialized data to.
-    iostream* myStream;
+    unique_ptr<iostream> myStream;
     bool myUseFilestream;
 
     enum {

src/emucore/StateManager.cxx

@@ -184,20 +184,19 @@ void StateManager::loadState(int slot)
     // First test if we have a valid header
     // If so, do a complete state load using the Console
     buf.str("");
-    if(in.getString() != STATE_HEADER)
+    try
-      buf << "Incompatible state " << slot << " file";
-    else
     {
-      if(in.getString() == myOSystem.console().cartridge().name())
+      if(in.getString() != STATE_HEADER)
-      {
+        buf << "Incompatible state " << slot << " file";
-        if(myOSystem.console().load(in))
+      else if(in.getString() != myOSystem.console().cartridge().name())
-          buf << "State " << slot << " loaded";
-        else
-          buf << "Invalid data in state " << slot << " file";
-      }
-      else
         buf << "State " << slot << " file doesn't match current ROM";
     }
+    catch(...) { /* fall through to logic below */ }
+
+    if(myOSystem.console().load(in))
+      buf << "State " << slot << " loaded";
+    else
+      buf << "Invalid data in state " << slot << " file";
 
     myOSystem.frameBuffer().showMessage(buf.str());
   }
@@ -225,12 +224,21 @@ void StateManager::saveState(int slot)
       return;
     }
 
-    // Add header so that if the state format changes in the future,
+    try
-    // we'll know right away, without having to parse the rest of the file
+    {
-    out.putString(STATE_HEADER);
+      // Add header so that if the state format changes in the future,
+      // we'll know right away, without having to parse the rest of the file
+      out.putString(STATE_HEADER);
 
-    // Sanity check; prepend the cart type/name
+      // Sanity check; prepend the cart type/name
-    out.putString(myOSystem.console().cartridge().name());
+      out.putString(myOSystem.console().cartridge().name());
+    }
+    catch(...)
+    {
+      buf << "Error saving state " << slot;
+      myOSystem.frameBuffer().showMessage(buf.str());
+      return;
+    }
 
     // Do a complete state save using the Console
     buf.str("");
@@ -264,18 +272,25 @@ void StateManager::changeState()
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 bool StateManager::loadState(Serializer& in)
 {
-  if(myOSystem.hasConsole())
+  try
   {
-    // Make sure the file can be opened for reading
+    if(myOSystem.hasConsole())
-    if(in.valid())
     {
-      // First test if we have a valid header and cart type
+      // Make sure the file can be opened for reading
-      // If so, do a complete state load using the Console
+      if(in.valid())
-      return in.getString() == STATE_HEADER &&
+      {
-             in.getString() == myOSystem.console().cartridge().name() &&
+        // First test if we have a valid header and cart type
-             myOSystem.console().load(in);
+        // If so, do a complete state load using the Console
+        return in.getString() == STATE_HEADER &&
+               in.getString() == myOSystem.console().cartridge().name() &&
+               myOSystem.console().load(in);
+      }
     }
   }
+  catch(...)
+  {
+    cerr << "ERROR: StateManager::loadState(Serializer&)" << endl;
+  }
   return false;
 }