Fix parsing large MVC files causing lockup/crash in GUI (fixes # 829).

Still TODO is deal with large files other than MVC, and also ZIP files.
At least the logic for analyzing size is now in one method (OSystem::openROM).

src/emucore/FSNode.cxx

@@ -351,8 +351,8 @@ size_t FilesystemNode::read(ByteBuffer& buffer, size_t size) const
 
     if (sizeRead == 0)
       throw runtime_error("Zero-byte file");
-    else if (size != 0)
-      sizeRead = size;
+    else if (size > 0)  // If a requested size to read is provided, honour it
+      sizeRead = std::min(sizeRead, size);
 
     buffer = make_unique<uInt8[]>(sizeRead);
     in.read(reinterpret_cast<char*>(buffer.get()), sizeRead);

src/emucore/MD5.cxx

@@ -317,7 +317,7 @@ string hash(const ByteBuffer& buffer, size_t length)
 // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
 string hash(const uInt8* buffer, size_t length)
 {
-  constexpr char hex[] = "0123456789abcdef";
+  static constexpr char hex[] = "0123456789abcdef";
   MD5_CTX context{};
   uInt8 md5[16] = {0};
   const uInt32 len32 = static_cast<uInt32>(length);  // Always use 32-bit for now
@@ -336,22 +336,4 @@ string hash(const uInt8* buffer, size_t length)
   return result;
 }
 
-// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
-string hash(const FilesystemNode& node)
-{
-  ByteBuffer image;
-  size_t size = 0;
-  try
-  {
-    size = node.read(image);
-  }
-  catch(...)
-  {
-    return EmptyString;
-  }
-
-  const string& md5 = hash(image, size);
-  return md5;
-}
-
 }  // Namespace MD5

src/emucore/MD5.hxx

@@ -33,29 +33,22 @@ namespace MD5 {
     Based on the size of data we currently use, this may never
     actually happen.
 
-    @param buffer The message to compute the digest of
-    @param length The length of the message
-    @return The message-digest
+    @param buffer  The message to compute the digest of
+    @param length  The length of the message
+
+    @return   The message-digest
   */
   string hash(const ByteBuffer& buffer, size_t length);
   string hash(const uInt8* buffer, size_t length);
   /**
     Dito.
 
-    @param buffer The message to compute the digest of
-    @return The message - digest
+    @param buffer  The message to compute the digest of
+
+    @return   The message - digest
   */
   string hash(const string& buffer);
 
-  /**
-    Get the MD5 Message-Digest of the file contained in 'node'.
-    The digest consists of 32 hexadecimal digits.
-
-    @param node The file node to compute the digest of
-    @return The message-digest
-  */
-  string hash(const FilesystemNode& node);
-
 }  // Namespace MD5
 
 #endif

src/emucore/OSystem.cxx

@@ -753,6 +753,34 @@ ByteBuffer OSystem::openROM(const FilesystemNode& rom, string& md5, size_t& size
   // but also adds a properties entry if the one for the ROM doesn't
   // contain a valid name
 
+  ByteBuffer image = openROM(rom, size);
+  if(image)
+  {
+    // If we get to this point, we know we have a valid file to open
+    // Now we make sure that the file has a valid properties entry
+    // To save time, only generate an MD5 if we really need one
+    if(md5 == "")
+      md5 = MD5::hash(image, size);
+
+    // Make sure to load a per-ROM properties entry, if one exists
+    myPropSet->loadPerROM(rom, md5);
+  }
+
+  return image;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+string OSystem::getROMMD5(const FilesystemNode& rom) const
+{
+  size_t size = 0;
+  const ByteBuffer image = openROM(rom, size);
+
+  return image ? MD5::hash(image, size) : EmptyString;
+}
+
+// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -
+ByteBuffer OSystem::openROM(const FilesystemNode& rom, size_t& size) const
+{
   // First check if this is a 'streaming' ROM (one where we only read
   // a portion of the file)
   const size_t sizeToRead = CartDetector::isProbablyMVC(rom);
@@ -762,17 +790,15 @@ ByteBuffer OSystem::openROM(const FilesystemNode& rom, string& md5, size_t& size
   //       if it's not a ZIP file (that size should be higher; still TBD)
 
   ByteBuffer image;
-  if((size = rom.read(image, sizeToRead)) == 0)
-    return nullptr;
-
-  // If we get to this point, we know we have a valid file to open
-  // Now we make sure that the file has a valid properties entry
-  // To save time, only generate an MD5 if we really need one
-  if(md5 == "")
-    md5 = MD5::hash(image, size);
-
-  // Make sure to load a per-ROM properties entry, if one exists
-  myPropSet->loadPerROM(rom, md5);
+  try
+  {
+    if((size = rom.read(image, sizeToRead)) == 0)
+      return nullptr;
+  }
+  catch(const runtime_error& e)
+  {
+    cerr << "ERROR: Couldn't open ROM (" << e.what() << ")";
+  }
 
   return image;
 }

src/emucore/OSystem.hxx

@@ -338,6 +338,15 @@ class OSystem
     */
     ByteBuffer openROM(const FilesystemNode& rom, string& md5, size_t& size);
 
+    /**
+      Open the given ROM and return the MD5sum of the data.
+
+      @param rom  The file node of the ROM to open (contains path)
+
+      @return  MD5 of the ROM image (if valid), otherwise EmptyString
+    */
+    string getROMMD5(const FilesystemNode& rom) const;
+
     /**
       Creates a new game console from the specified romfile, and correctly
       initializes the system state to start emulation of the Console.
@@ -596,7 +605,6 @@ class OSystem
     static bool ourOverrideBaseDirWithApp;
 
   private:
-
     /**
       This method should be called to initiate the process of loading settings
       from the config file.  It takes care of loading settings, applying
@@ -609,13 +617,24 @@ class OSystem
     */
     void createSound();
 
+    /**
+      Open the given ROM and return an array containing its contents.
+      This method takes care of using only a valid size for the
+
+      @param romfile  The file node of the ROM to open (contains path)
+      @param size     The amount of data read into the image array
+
+      @return  Unique pointer to the array, otherwise nullptr
+    */
+    ByteBuffer openROM(const FilesystemNode& romfile, size_t& size) const;
+
     /**
       Creates an actual Console object based on the given info.
 
       @param romfile  The file node of the ROM to use (contains path)
       @param md5      The MD5sum of the ROM
 
-      @return  The actual Console object, otherwise nullptr.
+      @return  The actual Console object, otherwise nullptr
     */
     unique_ptr<Console> openConsole(const FilesystemNode& romfile, string& md5);
 

src/gui/LauncherDialog.cxx

@@ -395,7 +395,7 @@ const string& LauncherDialog::selectedRomMD5()
   // Lookup MD5, and if not present, cache it
   const auto iter = myMD5List.find(currentNode().getPath());
   if(iter == myMD5List.end())
-    myMD5List[currentNode().getPath()] = MD5::hash(currentNode());
+    myMD5List[currentNode().getPath()] = instance().getROMMD5(currentNode());
 
   return myMD5List[currentNode().getPath()];
 }

src/gui/RomAuditDialog.cxx

@@ -139,7 +139,7 @@ void RomAuditDialog::auditRoms()
 
       // Calculate the MD5 so we can get the rest of the info
       // from the PropertiesSet (stella.pro)
-      const string& md5 = MD5::hash(files[idx]);
+      const string& md5 = instance().getROMMD5(files[idx]);
       if(instance().propSet().getMD5(md5, props))
       {
         const string& name = props.get(PropType::Cart_Name);