Hide ZipArchive implementation, only expose it via factory functions: makeZipArchive()

svn-id: r48040

6 files changed, 99 insertions, 83 deletions

diff --git a/backends/platform/psp/pspkeyboard.cpp b/backends/platform/psp/pspkeyboard.cpp
index 40b63d0cbd..e7e1dbb011 100644
--- a/backends/platform/psp/pspkeyboard.cpp
+++ b/backends/platform/psp/pspkeyboard.cpp
@@ -518,7 +518,7 @@ bool PSPKeyboard::load() {
 #ifdef PSP_KB_DEBUG
 		fprintf(stderr, "found kbd.zip\n");
 #endif
-		zipArchive  = new Common::ZipArchive(node.getChild("kbd.zip"));
+		zipArchive  = Common::makeZipArchive(node.getChild("kbd.zip"));
 	}
 
 	// Loop through different png images
diff --git a/backends/vkeybd/virtual-keyboard.cpp b/backends/vkeybd/virtual-keyboard.cpp
index 0e2b98a91d..bd512fe3ad 100644
--- a/backends/vkeybd/virtual-keyboard.cpp
+++ b/backends/vkeybd/virtual-keyboard.cpp
@@ -93,8 +93,8 @@ bool VirtualKeyboard::openPack(const String &packName, const FSNode &node) {
 #ifdef USE_ZLIB
 	if (node.getChild(packName + ".zip").exists()) {
 		// compressed keyboard pack
-		_fileArchive = new ZipArchive(node.getChild(packName + ".zip"));
-		if (_fileArchive->hasFile(packName + ".xml")) {
+		_fileArchive = makeZipArchive(node.getChild(packName + ".zip"));
+		if (_fileArchive && _fileArchive->hasFile(packName + ".xml")) {
 			if (!_parser->loadStream(_fileArchive->createReadStreamForMember(packName + ".xml"))) {
 				delete _fileArchive;
 				_fileArchive = 0;
diff --git a/common/unarj.h b/common/unarj.h
index 04f69e95f0..38450fa40a 100644
--- a/common/unarj.h
+++ b/common/unarj.h
@@ -34,6 +34,8 @@ class String;
 /**
  * This factory method creates an Archive instance corresponding to the content
  * of the ARJ compressed file with the given name.
+ *
+ * May return 0 in case of a failure.
  */
 Archive *makeArjArchive(const String &name);
 
diff --git a/common/unzip.cpp b/common/unzip.cpp
index 5c57736d4d..1db260d246 100644
--- a/common/unzip.cpp
+++ b/common/unzip.cpp
@@ -554,7 +554,7 @@ unzFile unzOpen(Common::SeekableReadStream *stream) {
 	if ((central_pos<us->offset_central_dir+us->size_central_dir) && (err==UNZ_OK))
 		err=UNZ_BADZIPFILE;
 
-	if (err!=UNZ_OK) {
+	if (err != UNZ_OK) {
 		delete us->_stream;
 		delete us;
 		return NULL;
@@ -576,12 +576,12 @@ unzFile unzOpen(Common::SeekableReadStream *stream) {
     these files MUST be closed with unzipCloseCurrentFile before call unzipClose.
   return UNZ_OK if there is no problem. */
 int unzClose(unzFile file) {
-	unz_s* s;
-	if (file==NULL)
+	unz_s *s;
+	if (file == NULL)
 		return UNZ_PARAMERROR;
-	s=(unz_s*)file;
+	s = (unz_s *)file;
 
-	if (s->pfile_in_zip_read!=NULL)
+	if (s->pfile_in_zip_read != NULL)
 		unzCloseCurrentFile(file);
 
 	delete s->_stream;
@@ -594,12 +594,12 @@ int unzClose(unzFile file) {
   Write info about the ZipFile in the *pglobal_info structure.
   No preparation of the structure is needed
   return UNZ_OK if there is no problem. */
-int unzGetGlobalInfo (unzFile file, unz_global_info *pglobal_info) {
-	unz_s* s;
-	if (file==NULL)
+int unzGetGlobalInfo(unzFile file, unz_global_info *pglobal_info) {
+	unz_s *s;
+	if (file == NULL)
 		return UNZ_PARAMERROR;
-	s=(unz_s*)file;
-	*pglobal_info=s->gi;
+	s = (unz_s *)file;
+	*pglobal_info = s->gi;
 	return UNZ_OK;
 }
 
@@ -607,7 +607,7 @@ int unzGetGlobalInfo (unzFile file, unz_global_info *pglobal_info) {
 /*
    Translate date/time from Dos format to tm_unz (readable more easilty)
 */
-static void unzlocal_DosDateToTmuDate (uLong ulDosDate, tm_unz* ptm) {
+static void unzlocal_DosDateToTmuDate(uLong ulDosDate, tm_unz* ptm) {
 	uLong uDate;
 	uDate = (uLong)(ulDosDate>>16);
 	ptm->tm_mday = (uInt)(uDate&0x1f) ;
@@ -1293,12 +1293,12 @@ int unzCloseCurrentFile(unzFile file) {
 
 	unz_s* s;
 	file_in_zip_read_info_s* pfile_in_zip_read_info;
-	if (file==NULL)
+	if (file == NULL)
 		return UNZ_PARAMERROR;
-	s=(unz_s*)file;
-	pfile_in_zip_read_info=s->pfile_in_zip_read;
+	s = (unz_s*)file;
+	pfile_in_zip_read_info = s->pfile_in_zip_read;
 
-	if (pfile_in_zip_read_info==NULL)
+	if (pfile_in_zip_read_info == NULL)
 		return UNZ_PARAMERROR;
 
 
@@ -1357,6 +1357,21 @@ int unzGetGlobalComment(unzFile file, char *szComment, uLong uSizeBuf) {
 namespace Common {
 
 
+class ZipArchive : public Archive {
+	unzFile _zipFile;
+
+public:
+	ZipArchive(unzFile zipFile);
+
+
+	~ZipArchive();
+
+	virtual bool hasFile(const String &name);
+	virtual int listMembers(ArchiveMemberList &list);
+	virtual ArchiveMemberPtr getMember(const String &name);
+	virtual SeekableReadStream *createReadStreamForMember(const String &name) const;
+};
+
 /*
 class ZipArchiveMember : public ArchiveMember {
 	unzFile _zipFile;
@@ -1375,36 +1390,19 @@ public:
 };
 */
 
-ZipArchive::ZipArchive(const Common::String &name) {
-	SeekableReadStream *stream = SearchMan.createReadStreamForMember(name);
-	_zipFile = unzOpen(stream);
-}
-
-ZipArchive::ZipArchive(const Common::FSNode &node) {
-	SeekableReadStream *stream = node.createReadStream();
-	_zipFile = unzOpen(stream);
-}
-
-ZipArchive::ZipArchive(Common::SeekableReadStream *stream) {
-	_zipFile = unzOpen(stream);
+ZipArchive::ZipArchive(unzFile zipFile) : _zipFile(zipFile) {
+	assert(_zipFile);
 }
 
 ZipArchive::~ZipArchive() {
 	unzClose(_zipFile);
 }
 
-bool ZipArchive::isOpen() const {
-	return _zipFile != 0;
-}
-
 bool ZipArchive::hasFile(const Common::String &name) {
-	return (_zipFile && unzLocateFile(_zipFile, name.c_str(), 2) == UNZ_OK);
+	return (unzLocateFile(_zipFile, name.c_str(), 2) == UNZ_OK);
 }
 
 int ZipArchive::listMembers(Common::ArchiveMemberList &list) {
-	if (!_zipFile)
-		return 0;
-
 	int matches = 0;
 	int err = unzGoToFirstFile(_zipFile);
 
@@ -1422,16 +1420,13 @@ int ZipArchive::listMembers(Common::ArchiveMemberList &list) {
 }
 
 ArchiveMemberPtr ZipArchive::getMember(const String &name) {
-	if (!_zipFile || !hasFile(name))
+	if (!hasFile(name))
 		return ArchiveMemberPtr();
 
 	return ArchiveMemberPtr(new GenericArchiveMember(name, this));
 }
 
 Common::SeekableReadStream *ZipArchive::createReadStreamForMember(const Common::String &name) const {
-	if (!_zipFile)
-		return 0;
-
 	if (unzLocateFile(_zipFile, name.c_str(), 2) != UNZ_OK)
 		return 0;
 
@@ -1450,7 +1445,25 @@ Common::SeekableReadStream *ZipArchive::createReadStreamForMember(const Common::
 	// files in the archive and tries to use them indepenendtly.
 }
 
-}	// End of namespace Common
+Archive *makeZipArchive(const String &name) {
+	return makeZipArchive(SearchMan.createReadStreamForMember(name));
+}
+
+Archive *makeZipArchive(const FSNode &node) {
+	return makeZipArchive(node.createReadStream());
+}
+
+Archive *makeZipArchive(SeekableReadStream *stream) {
+	if (!stream)
+		return 0;
+	unzFile zipFile = unzOpen(stream);
+	if (!zipFile) {
+		delete stream;
+		return 0;
+	}
+	return new ZipArchive(zipFile);
+}
 
+}	// End of namespace Common
 
 #endif
diff --git a/common/unzip.h b/common/unzip.h
index 94d098208a..2f87a96d2b 100644
--- a/common/unzip.h
+++ b/common/unzip.h
@@ -27,46 +27,41 @@
 
 #ifdef USE_ZLIB
 
-#include "common/scummsys.h"
-#include "common/archive.h"
-
-typedef void *unzFile;
-
 namespace Common {
 
-class ZipArchive : public Archive {
-	void *_zipFile;
-
-public:
-	/**
-	 * Open the .zip archive with the given file name.
-	 */
-	ZipArchive(const String &name);
-
-	/**
-	 * Open the .zip archive to which the given FSNode refers to.
-	 */
-	ZipArchive(const FSNode &node);
-
-	/**
-	 * Open a .zip file from a stream. This takes ownership of the stream,
-	 * in particular, it is closed when the ZipArchive is deleted.
-	 */
-	ZipArchive(SeekableReadStream *stream);
-
+class Archive;
+class FSNode;
+class SeekableReadStream;
+class String;
 
-	~ZipArchive();
+/**
+ * This factory method creates an Archive instance corresponding to the content
+ * of the ZIP compressed file with the given name.
+ *
+ * May return 0 in case of a failure.
+ */
+Archive *makeZipArchive(const String &name);
 
-	bool isOpen() const;
+/**
+ * This factory method creates an Archive instance corresponding to the content
+ * of the ZIP compressed file with the given name.
+ *
+ * May return 0 in case of a failure.
+ */
+Archive *makeZipArchive(const FSNode &node);
 
-	virtual bool hasFile(const String &name);
-	virtual int listMembers(ArchiveMemberList &list);
-	virtual ArchiveMemberPtr getMember(const String &name);
-	virtual SeekableReadStream *createReadStreamForMember(const String &name) const;
-};
+/**
+ * This factory method creates an Archive instance corresponding to the content
+ * of the given ZIP compressed datastream.
+ * This takes ownership of the stream,  in particular, it is deleted when the
+ * ZipArchive is deleted.
+ *
+ * May return 0 in case of a failure. In this case stream will still be deleted.
+ */
+Archive *makeZipArchive(SeekableReadStream *stream);
 
 }	// End of namespace Common
 
 #endif // USE_ZLIB
 
-#endif /* _unz_H */
+#endif
diff --git a/gui/ThemeEngine.cpp b/gui/ThemeEngine.cpp
index a08e019668..fb2603eb16 100644
--- a/gui/ThemeEngine.cpp
+++ b/gui/ThemeEngine.cpp
@@ -393,11 +393,9 @@ bool ThemeEngine::init() {
 		Common::FSNode node(_themeFile);
 		if (node.getName().hasSuffix(".zip") && !node.isDirectory()) {
 #ifdef USE_ZLIB
-			Common::ZipArchive *zipArchive = new Common::ZipArchive(node);
+			Common::Archive *zipArchive = Common::makeZipArchive(node);
 
-			if (!zipArchive || !zipArchive->isOpen()) {
-				delete zipArchive;
-				zipArchive = 0;
+			if (!zipArchive) {
 				warning("Failed to open Zip archive '%s'.", node.getPath().c_str());
 			}
 			_themeArchive = zipArchive;
@@ -1444,10 +1442,18 @@ bool ThemeEngine::themeConfigUsable(const Common::FSNode &node, Common::String &
 
 	if (node.getName().hasSuffix(".zip") && !node.isDirectory()) {
 #ifdef USE_ZLIB
-		Common::ZipArchive zipArchive(node);
-		if (zipArchive.hasFile("THEMERC")) {
-			stream.open("THEMERC", zipArchive);
+		Common::Archive *zipArchive = Common::makeZipArchive(node);
+		if (zipArchive && zipArchive->hasFile("THEMERC")) {
+			// Open THEMERC from the ZIP file.
+			stream.open("THEMERC", *zipArchive);
 		}
+		// Delete the ZIP archive again. Note: This only works because
+		// stream.open() only uses ZipArchive::createReadStreamForMember,
+		// and that in turn happens to read all the data for a given
+		// archive member into a memory block. So there will be no dangling
+		// reference to zipArchive anywhere. This could change if we
+		// ever modify ZipArchive::createReadStreamForMember.
+		delete zipArchive;
 #endif
 	} else if (node.isDirectory()) {
 		Common::FSNode headerfile = node.getChild("THEMERC");