diff --git a/Changes.txt b/Changes.txt index d87479232..a5ed881b4 100644 --- a/Changes.txt +++ b/Changes.txt @@ -14,6 +14,9 @@ 4.6.1 to 4.7: (mmm dd, 2015) + * Fixed memory leak; the game console wasn't being closed after exiting + a ROM. + * Updated included PNG library to latest stable version. -Have fun! diff --git a/src/common/FSNodeZIP.cxx b/src/common/FSNodeZIP.cxx index 774feee44..d7ac74d56 100644 --- a/src/common/FSNodeZIP.cxx +++ b/src/common/FSNodeZIP.cxx @@ -175,7 +175,7 @@ bool FilesystemNodeZIP::getChildren(AbstractFSList& myList, ListMode mode, } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt32 FilesystemNodeZIP::read(uInt8*& image) const +uInt32 FilesystemNodeZIP::read(BytePtr& image) const { switch(_error) { diff --git a/src/common/FSNodeZIP.hxx b/src/common/FSNodeZIP.hxx index 0633d968b..1da8a5d30 100644 --- a/src/common/FSNodeZIP.hxx +++ b/src/common/FSNodeZIP.hxx @@ -66,7 +66,7 @@ class FilesystemNodeZIP : public AbstractFSNode bool getChildren(AbstractFSList& list, ListMode mode, bool hidden) const; AbstractFSNode* getParent() const; - uInt32 read(uInt8*& image) const; + uInt32 read(BytePtr& image) const; private: FilesystemNodeZIP(const string& zipfile, const string& virtualpath, diff --git a/src/common/ZipHandler.cxx b/src/common/ZipHandler.cxx index 40a99ba88..d3c97ba64 100644 --- a/src/common/ZipHandler.cxx +++ b/src/common/ZipHandler.cxx @@ -87,7 +87,7 @@ string ZipHandler::next() } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt32 ZipHandler::decompress(uInt8*& image) +uInt32 ZipHandler::decompress(BytePtr& image) { static const char* zip_error_s[] = { "ZIPERR_NONE", @@ -104,17 +104,13 @@ uInt32 ZipHandler::decompress(uInt8*& image) if(myZip) { uInt32 length = myZip->header.uncompressed_length; - image = new uInt8[length]; + image = make_ptr(length); - ZipHandler::zip_error err = zip_file_decompress(myZip, image, length); + ZipHandler::zip_error err = zip_file_decompress(myZip, image.get(), length); if(err == ZIPERR_NONE) return length; else - { - delete[] image; image = nullptr; - throw runtime_error(zip_error_s[err]); - } } else throw runtime_error("Invalid ZIP archive"); diff --git a/src/common/ZipHandler.hxx b/src/common/ZipHandler.hxx index 417f364e9..6b9c15577 100644 --- a/src/common/ZipHandler.hxx +++ b/src/common/ZipHandler.hxx @@ -81,7 +81,7 @@ class ZipHandler // Decompress the currently selected file and return its length // An exception will be thrown on any errors - uInt32 decompress(uInt8*& image); + uInt32 decompress(BytePtr& image); // Answer the number of ROM files found in the archive // Currently, this means files with extension a26/bin/rom diff --git a/src/common/bspf.hxx b/src/common/bspf.hxx index acb4705f5..9f3908f8d 100644 --- a/src/common/bspf.hxx +++ b/src/common/bspf.hxx @@ -63,6 +63,7 @@ using IntArray = vector; using BoolArray = vector; using ByteArray = vector; using StringList = vector; +using BytePtr = unique_ptr; // Defines to help with path handling #if (defined(BSPF_UNIX) || defined(BSPF_MAC_OSX)) diff --git a/src/emucore/Cart.cxx b/src/emucore/Cart.cxx index b65ba03c0..09c24fe0a 100644 --- a/src/emucore/Cart.cxx +++ b/src/emucore/Cart.cxx @@ -70,12 +70,17 @@ #endif // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -Cartridge* Cartridge::create(const uInt8* image, uInt32 size, string& md5, - string& dtype, string& id, const OSystem& osystem, Settings& settings) +unique_ptr Cartridge::create(const BytePtr& img, uInt32 size, + string& md5, string& dtype, string& id, + const OSystem& osystem, Settings& settings) { - Cartridge* cartridge = nullptr; + unique_ptr cartridge; string type = dtype; + // For now, we convert from BytePtr to the raw pointer + // Eventually, the Cartridge hierarchy should probably use BytePtr directly + const uInt8* image = img.get(); + // Collect some info about the ROM ostringstream buf; @@ -181,81 +186,81 @@ Cartridge* Cartridge::create(const uInt8* image, uInt32 size, string& md5, // We should know the cart's type by now so let's create it if(type == "0840") - cartridge = new Cartridge0840(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "2K") - cartridge = new Cartridge2K(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "3E") - cartridge = new Cartridge3E(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "3F") - cartridge = new Cartridge3F(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "4A50") - cartridge = new Cartridge4A50(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "4K") - cartridge = new Cartridge4K(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "4KSC") - cartridge = new Cartridge4KSC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "AR") - cartridge = new CartridgeAR(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "CM") - cartridge = new CartridgeCM(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "CTY") - cartridge = new CartridgeCTY(image, size, osystem); + cartridge = make_ptr(image, size, osystem); else if(type == "CV") - cartridge = new CartridgeCV(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "DASH") - cartridge = new CartridgeDASH(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "DPC") - cartridge = new CartridgeDPC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "DPC+") - cartridge = new CartridgeDPCPlus(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "E0") - cartridge = new CartridgeE0(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "E7") - cartridge = new CartridgeE7(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "EF") - cartridge = new CartridgeEF(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "EFSC") - cartridge = new CartridgeEFSC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "BF") - cartridge = new CartridgeBF(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "BFSC") - cartridge = new CartridgeBFSC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "DF") - cartridge = new CartridgeDF(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "DFSC") - cartridge = new CartridgeDFSC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F0" || type == "MB") - cartridge = new CartridgeF0(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F4") - cartridge = new CartridgeF4(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F4SC") - cartridge = new CartridgeF4SC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F6") - cartridge = new CartridgeF6(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F6SC") - cartridge = new CartridgeF6SC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "F8") - cartridge = new CartridgeF8(image, size, md5, settings); + cartridge = make_ptr(image, size, md5, settings); else if(type == "F8SC") - cartridge = new CartridgeF8SC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "FA" || type == "FASC") - cartridge = new CartridgeFA(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "FA2") - cartridge = new CartridgeFA2(image, size, osystem); + cartridge = make_ptr(image, size, osystem); else if(type == "FE") - cartridge = new CartridgeFE(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "MC") - cartridge = new CartridgeMC(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "MDM") - cartridge = new CartridgeMDM(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "UA") - cartridge = new CartridgeUA(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "SB") - cartridge = new CartridgeSB(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "WD") - cartridge = new CartridgeWD(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(type == "X07") - cartridge = new CartridgeX07(image, size, settings); + cartridge = make_ptr(image, size, settings); else if(dtype == "WRONG_SIZE") throw runtime_error("Invalid cart size for type '" + type + "'"); else diff --git a/src/emucore/Cart.hxx b/src/emucore/Cart.hxx index 0495fc3ff..abc82c5ff 100644 --- a/src/emucore/Cart.hxx +++ b/src/emucore/Cart.hxx @@ -60,9 +60,10 @@ class Cartridge : public Device @param settings The settings associated with the system @return Pointer to the new cartridge object allocated on the heap */ - static Cartridge* create(const uInt8* image, uInt32 size, string& md5, - string& dtype, string& id, - const OSystem& system, Settings& settings); + static unique_ptr + create(const BytePtr& image, uInt32 size, + string& md5, string& dtype, string& id, + const OSystem& system, Settings& settings); /** Create a new cartridge @@ -257,6 +258,7 @@ class Cartridge : public Device @param image A pointer to the ROM image @param size The size of the ROM image + @return The "best guess" for the cartridge type */ static string autodetectType(const uInt8* image, uInt32 size); diff --git a/src/emucore/Console.cxx b/src/emucore/Console.cxx index 88ce3cab0..ad8d5b536 100644 --- a/src/emucore/Console.cxx +++ b/src/emucore/Console.cxx @@ -66,10 +66,12 @@ #include "Console.hxx" // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -Console::Console(OSystem& osystem, Cartridge* cart, const Properties& props) +Console::Console(OSystem& osystem, unique_ptr& cart, + const Properties& props) : myOSystem(osystem), myEvent(osystem.eventHandler().event()), myProperties(props), + myCart(std::move(cart)), myDisplayFormat(""), // Unknown TV format @ start myFramerate(0.0), // Unknown framerate @ start myCurrentFormat(0), // Unknown format @ start @@ -82,7 +84,6 @@ Console::Console(OSystem& osystem, Cartridge* cart, const Properties& props) my6502 = make_ptr(myOSystem.settings()); myRiot = make_ptr(*this, myOSystem.settings()); myTIA = make_ptr(*this, myOSystem.sound(), myOSystem.settings()); - myCart = unique_ptr(cart); mySwitches = make_ptr(myEvent, myProperties); // Construct the system and components diff --git a/src/emucore/Console.hxx b/src/emucore/Console.hxx index 4955646df..9d198d300 100644 --- a/src/emucore/Console.hxx +++ b/src/emucore/Console.hxx @@ -69,7 +69,8 @@ class Console : public Serializable @param cart The cartridge to use with this console @param props The properties for the cartridge */ - Console(OSystem& osystem, Cartridge* cart, const Properties& props); + Console(OSystem& osystem, unique_ptr& cart, + const Properties& props); /** Destructor diff --git a/src/emucore/FSNode.cxx b/src/emucore/FSNode.cxx index 903e97f0f..f2a2e137e 100644 --- a/src/emucore/FSNode.cxx +++ b/src/emucore/FSNode.cxx @@ -175,7 +175,7 @@ bool FilesystemNode::rename(const string& newfile) } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt32 FilesystemNode::read(uInt8*& image) const +uInt32 FilesystemNode::read(BytePtr& image) const { uInt32 size = 0; @@ -191,15 +191,13 @@ uInt32 FilesystemNode::read(uInt8*& image) const gzFile f = gzopen(getPath().c_str(), "rb"); if(f) { - image = new uInt8[512 * 1024]; - size = gzread(f, image, 512 * 1024); + image = make_ptr(512 * 1024); + size = gzread(f, image.get(), 512 * 1024); gzclose(f); if(size == 0) - { - delete[] image; image = nullptr; throw runtime_error("Zero-byte file"); - } + return size; } else diff --git a/src/emucore/FSNode.hxx b/src/emucore/FSNode.hxx index cfe289942..8b4712128 100644 --- a/src/emucore/FSNode.hxx +++ b/src/emucore/FSNode.hxx @@ -229,14 +229,13 @@ class FilesystemNode /** * Read data (binary format) into the given buffer. * - * @param buffer The buffer to containing the data - * This will be allocated by the method, and must be - * freed by the caller. + * @param buffer The buffer to contain the data. + * * @return The number of bytes read (0 in the case of failure) * This method can throw exceptions, and should be used inside * a try-catch block. */ - virtual uInt32 read(uInt8*& buffer) const; + virtual uInt32 read(BytePtr& buffer) const; /** * The following methods are almost exactly the same as the various @@ -380,7 +379,7 @@ class AbstractFSNode * This method can throw exceptions, and should be used inside * a try-catch block. */ - virtual uInt32 read(uInt8*& buffer) const { return 0; } + virtual uInt32 read(BytePtr& buffer) const { return 0; } /** * The parent node of this directory. diff --git a/src/emucore/MD5.cxx b/src/emucore/MD5.cxx index bc92d0cf5..d8aaea0e3 100644 --- a/src/emucore/MD5.cxx +++ b/src/emucore/MD5.cxx @@ -325,6 +325,12 @@ static void MD5_memset(POINTER output, int value, uInt32 len) ((char *)output)[i] = (char)value; } +// - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - +string hash(const BytePtr& buffer, uInt32 length) +{ + return hash(buffer.get(), length); +} + // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - string hash(const uInt8* buffer, uInt32 length) { @@ -349,7 +355,7 @@ string hash(const uInt8* buffer, uInt32 length) // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - string hash(const FilesystemNode& node) { - uInt8* image = nullptr; + BytePtr image; uInt32 size = 0; try { @@ -361,7 +367,6 @@ string hash(const FilesystemNode& node) } const string& md5 = hash(image, size); - delete[] image; return md5; } diff --git a/src/emucore/MD5.hxx b/src/emucore/MD5.hxx index 907fcf533..34f31cfbc 100644 --- a/src/emucore/MD5.hxx +++ b/src/emucore/MD5.hxx @@ -33,6 +33,7 @@ namespace MD5 { @param length The length of the message @return The message-digest */ +string hash(const BytePtr& buffer, uInt32 length); string hash(const uInt8* buffer, uInt32 length); /** diff --git a/src/emucore/OSystem.cxx b/src/emucore/OSystem.cxx index 3bb29b26c..7d575b836 100644 --- a/src/emucore/OSystem.cxx +++ b/src/emucore/OSystem.cxx @@ -473,9 +473,9 @@ unique_ptr OSystem::openConsole(const FilesystemNode& romfile, unique_ptr console; // Open the cartridge image and read it in - uInt8* image = nullptr; + BytePtr image; uInt32 size = 0; - if((image = openROM(romfile, md5, size)) != 0) + if((image = openROM(romfile, md5, size)) != nullptr) { // Get a valid set of properties, including any entered on the commandline // For initial creation of the Cart, we're only concerned with the BS type @@ -494,7 +494,7 @@ unique_ptr OSystem::openConsole(const FilesystemNode& romfile, // Now create the cartridge string cartmd5 = md5; type = props.get(Cartridge_Type); - Cartridge* cart = + unique_ptr cart = Cartridge::create(image, size, cartmd5, type, id, *this, *mySettings); // It's possible that the cart created was from a piece of the image, @@ -532,10 +532,6 @@ unique_ptr OSystem::openConsole(const FilesystemNode& romfile, console = make_ptr(*this, cart, props); } - // Free the image since we don't need it any longer - if(image != 0 && size > 0) - delete[] image; - return console; } @@ -548,24 +544,20 @@ void OSystem::closeConsole() // If a previous console existed, save cheats before creating a new one myCheatManager->saveCheats(myConsole->properties().get(Cartridge_MD5)); #endif - myConsole.release(); } } // - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - -uInt8* OSystem::openROM(const FilesystemNode& rom, string& md5, uInt32& size) +BytePtr OSystem::openROM(const FilesystemNode& rom, string& md5, uInt32& size) { // This method has a documented side-effect: // It not only loads a ROM and creates an array with its contents, // but also adds a properties entry if the one for the ROM doesn't // contain a valid name - uInt8* image = nullptr; + BytePtr image; if((size = rom.read(image)) == 0) - { - delete[] image; 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 diff --git a/src/emucore/OSystem.hxx b/src/emucore/OSystem.hxx index 886602f91..a88f6bec8 100644 --- a/src/emucore/OSystem.hxx +++ b/src/emucore/OSystem.hxx @@ -20,7 +20,6 @@ #ifndef OSYSTEM_HXX #define OSYSTEM_HXX -class Cartridge; class CheatManager; class CommandMenu; class Console; @@ -36,6 +35,7 @@ class Sound; class StateManager; class VideoDialog; +#include "Cart.hxx" #include "FSNode.hxx" #include "FrameBuffer.hxx" #include "PNGLibrary.hxx" @@ -558,10 +558,9 @@ class OSystem (will be recalculated if necessary) @param size The amount of data read into the image array - @return Pointer to the array, with size >=0 indicating valid data - (calling method is responsible for deleting it) + @return Unique pointer to the array */ - uInt8* openROM(const FilesystemNode& rom, string& md5, uInt32& size); + BytePtr openROM(const FilesystemNode& rom, string& md5, uInt32& size); /** Gets all possible info about the given console.