Merge lp:~widelands-dev/widelands/fix_zip_filesystem into lp:widelands

[SirVer]

Use std::shared_ptr to properly share zip files between classes.

ZipFilesystem, any child filesystems created by make_sub_file_system, and any ZipStream[Write|Read] created from such a filesystem access the same physical zip file and corresponding state. If one wrote while the other read, the outcome was undefined.

This changes the code to move the zip file abstraction into a wrapper class that properly reopens files for the correct access.

This also get's rid of the improper (and deprecated) use of the implicit copy constructor - the state was copied, but not properly shared before.

Fix deprecated use of implicit copy constructor and the mess that ZipFilesystem was in general.

[SirVer (sirver)]

Testers: This should be a refactoring only to make sure we are not broken by c++14 (which deprecates our inproper use of Copy construction in this case). No noticeable changes in behavior.

[bunnybot (widelandsofficial)]

Hi, I am bunnybot (https://github.com/widelands/bunnybot).

I am keeping the source branch lp:~widelands-dev/widelands/fix_zip_filesystem mirrored to https://github.com/widelands/widelands/tree/_widelands_dev_widelands_fix_zip_filesystem

You can give me commands by starting a line with @bunnybot <command>. I understand:
 merge: Merges the source branch into the target branch, closing the merge proposal. I will use the proposed commit message if it is set.

[bunnybot (widelandsofficial)]

Travis build 295 has changed state to: passed. Details: https://travis-ci.org/widelands/widelands/builds/103019352.

[TiborB (tiborb95)]

Tested - briefly - no issues

code looks good to me as far as I can tell

review: Approve

[SirVer (sirver)]

Thanks!

@bunnybot merge

=== modified file 'src/io/filesystem/zip_filesystem.cc'
--- src/io/filesystem/zip_filesystem.cc	2015-10-25 15:44:36 +0000
+++ src/io/filesystem/zip_filesystem.cc	2016-01-17 21:52:36 +0000
@@ -35,28 +35,99 @@
 #include "io/streamread.h"
 #include "io/streamwrite.h"
 
+ZipFilesystem::ZipFile::ZipFile(const std::string& zipfile)
+   : state_(State::kIdle),
+     path_(zipfile),
+     basename_(fs_filename(zipfile.c_str())),
+     write_handle_(nullptr),
+     read_handle_(nullptr) {
+}
+
+ZipFilesystem::ZipFile::~ZipFile() {
+	close();
+}
+
+void ZipFilesystem::ZipFile::close() {
+	if (state_ == State::kZipping) {
+		zipClose(write_handle_, nullptr);
+	} else if (state_ == State::kUnzipping) {
+		unzClose(read_handle_);
+	}
+	state_ = State::kIdle;
+}
+
+void ZipFilesystem::ZipFile::open_for_zip() {
+	if (state_ == State::kZipping)
+		return;
+
+	close();
+
+	write_handle_ = zipOpen(path_.c_str(), APPEND_STATUS_ADDINZIP);
+	if (!write_handle_) {
+		//  Couldn't open for append, so create new.
+		write_handle_ = zipOpen(path_.c_str(), APPEND_STATUS_CREATE);
+	}
+
+	state_ = State::kZipping;
+}
+
+void ZipFilesystem::ZipFile::open_for_unzip() {
+	if (state_ == State::kUnzipping)
+		return;
+
+	close();
+
+	read_handle_ = unzOpen(path_.c_str());
+	if (!read_handle_)
+		throw FileTypeError
+			("ZipFilesystem::open_for_unzip", path_, "not a .zip file");
+	state_ = State::kUnzipping;
+}
+
+std::string ZipFilesystem::ZipFile::strip_basename(const std::string& filename) {
+	if (filename.compare(0, basename_.length(), basename_) == 0)
+	{
+		// filename contains the name of the zip file as first element. This means
+		// this is an old zip file where all data were in a directory named as the
+		// file inside the zip file.
+		// return the filename without the first element
+		return filename.substr(basename_.length() + 1);
+	}
+
+	// seems to be a new zipfile without directory or a old zipfile was renamed
+	if (*filename.begin() == '/')
+		return filename.substr(1);
+	return filename;
+}
+
+const zipFile& ZipFilesystem::ZipFile::write_handle() {
+	open_for_zip();
+	return write_handle_;
+}
+
+const unzFile& ZipFilesystem::ZipFile::read_handle() {
+	open_for_unzip();
+	return read_handle_;
+}
+
+const std::string& ZipFilesystem::ZipFile::path() const {
+	return path_;
+}
+
 /**
  * Initialize the real file-system
  */
-ZipFilesystem::ZipFilesystem(const std::string & zipfile)
-:
-m_state      (STATE_IDLE),
-m_zipfile    (nullptr),
-m_unzipfile  (nullptr),
-m_oldzip     (false),
-m_zipfilename(zipfile),
-m_basenamezip(fs_filename(zipfile.c_str())),
-m_basename   ()
-{
+ZipFilesystem::ZipFilesystem(const std::string& zipfile)
+   : zip_file_(new ZipFile(zipfile)), basedir_in_zip_file_() {
 	// TODO(unknown): check OS permissions on whether the file is writable
 }
 
-/**
- * Cleanup code
- */
-ZipFilesystem::~ZipFilesystem()
-{
-	m_close();
+ZipFilesystem::ZipFilesystem(const std::shared_ptr<ZipFile>& shared_data,
+                             const std::string& basedir_in_zip_file)
+   : zip_file_(shared_data), basedir_in_zip_file_(basedir_in_zip_file) {
+}
+
+ZipFilesystem::~ZipFilesystem() {
 }
 
 /**
@@ -72,11 +143,9 @@
  * cross-platform way of doing this
  */
 std::set<std::string> ZipFilesystem::list_directory(const std::string& path_in) {
-	m_open_unzip();
-
 	assert(path_in.size()); //  prevent invalid read below
 
-	std::string path = m_basename;
+	std::string path = basedir_in_zip_file_;
 	if (*path_in.begin() != '/')
 		path += "/";
 	path += path_in;
@@ -85,18 +154,18 @@
 	if (*path.begin() == '/')
 		path = path.substr(1);
 
-	unzCloseCurrentFile(m_unzipfile);
-	unzGoToFirstFile(m_unzipfile);
+	unzCloseCurrentFile(zip_file_->read_handle());
+	unzGoToFirstFile(zip_file_->read_handle());
 
 	unz_file_info file_info;
 	char filename_inzip[256];
 	std::set<std::string> results;
 	for (;;) {
 		unzGetCurrentFileInfo
-			(m_unzipfile, &file_info, filename_inzip, sizeof(filename_inzip),
+			(zip_file_->read_handle(), &file_info, filename_inzip, sizeof(filename_inzip),
 			 nullptr, 0, nullptr, 0);
 
-		std::string complete_filename = strip_basename(filename_inzip);
+		std::string complete_filename = zip_file_->strip_basename(filename_inzip);
 		std::string filename = fs_filename(complete_filename.c_str());
 		std::string filepath =
 			complete_filename.substr
@@ -108,9 +177,9 @@
 		if
 			(('/' + path == filepath || path == filepath || path.length() == 1)
 			 && filename.size())
-		results.insert(complete_filename.substr(m_basename.size()));
+		results.insert(complete_filename.substr(basedir_in_zip_file_.size()));
 
-		if (unzGoToNextFile(m_unzipfile) == UNZ_END_OF_LIST_OF_FILE)
+		if (unzGoToNextFile(zip_file_->read_handle()) == UNZ_END_OF_LIST_OF_FILE)
 			break;
 	}
 	return results;
@@ -120,18 +189,19 @@
  * Returns true if the given file exists, and false if it doesn't.
  * Also returns false if the pathname is invalid
  */
-bool ZipFilesystem::file_exists(const std::string & path) {
+bool ZipFilesystem::file_exists(const std::string& path) {
 	try {
-		m_open_unzip(); //  TODO(unknown): check return value
+		unzGoToFirstFile(zip_file_->read_handle());
 	} catch (...) {
+		// The zip file could not be opened. I guess this means 'path' does not
+		// exist.
 		return false;
 	}
-	unzGoToFirstFile(m_unzipfile);
 	unz_file_info file_info;
 	char filename_inzip[256];
 	memset(filename_inzip, ' ', 256);
 
-	std::string path_in = m_basename + "/" + path;
+	std::string path_in = basedir_in_zip_file_ + "/" + path;
 
 	if (*path_in.begin() == '/')
 		path_in = path_in.substr(1);
@@ -140,10 +210,10 @@
 
 	for (;;) {
 		unzGetCurrentFileInfo
-			(m_unzipfile, &file_info, filename_inzip, sizeof(filename_inzip),
+			(zip_file_->read_handle(), &file_info, filename_inzip, sizeof(filename_inzip),
 			 nullptr, 0, nullptr, 0);
 
-		std::string complete_filename = strip_basename(filename_inzip);
+		std::string complete_filename = zip_file_->strip_basename(filename_inzip);
 
 		if (*complete_filename.rbegin() == '/')
 			complete_filename.resize(complete_filename.size() - 1);
@@ -151,7 +221,7 @@
 		if (path_in == complete_filename)
 			return true;
 
-		if (unzGoToNextFile(m_unzipfile) == UNZ_END_OF_LIST_OF_FILE)
+		if (unzGoToNextFile(zip_file_->read_handle()) == UNZ_END_OF_LIST_OF_FILE)
 			break;
 	}
 	return false;
@@ -170,7 +240,7 @@
 	char filename_inzip[256];
 
 	unzGetCurrentFileInfo
-		(m_unzipfile, &file_info, filename_inzip, sizeof(filename_inzip),
+		(zip_file_->read_handle(), &file_info, filename_inzip, sizeof(filename_inzip),
 		 nullptr, 0, nullptr, 0);
 
 	return filename_inzip[strlen(filename_inzip) - 1] == '/';
@@ -180,8 +250,6 @@
  * Create a sub filesystem out of this filesystem
  */
 FileSystem * ZipFilesystem::make_sub_file_system(const std::string & path) {
-	m_open_unzip();
-
 	if (!file_exists(path)) {
 		throw wexception("ZipFilesystem::make_sub_file_system: The path does not exist.");
 	}
@@ -189,17 +257,11 @@
 		throw wexception("ZipFilesystem::make_sub_file_system: The path needs to be a directory.");
 	}
 
-	m_close();
-
 	std::string localpath = path;
-
-	if (*localpath.begin() == '/')
+	if (*localpath.begin() == '/') {
 		localpath = localpath.substr(1);
-
-	ZipFilesystem * newfs = new ZipFilesystem(m_zipfilename);
-	newfs->m_basename = m_basename + "/" + localpath;
-
-	return newfs;
+	}
+	return new ZipFilesystem(zip_file_, basedir_in_zip_file_ + "/" + localpath);
 }
 
 /**
@@ -217,23 +279,15 @@
 	if (type != FileSystem::DIR)
 		throw ZipOperationError
 			("ZipFilesystem::create_sub_file_system",
-			 path, m_zipfilename,
+			 path, zip_file_->path(),
 			 "can not create ZipFilesystem inside another ZipFilesystem");
 
 	ensure_directory_exists(path);
 
-	m_close();
-
 	std::string localpath = path;
-
 	if (*localpath.begin() == '/')
 		localpath = localpath.substr(1);
-
-	ZipFilesystem * newfs = new ZipFilesystem(*this);
-
-	newfs->m_basename = m_basename + "/" + localpath;
-
-	return newfs;
+	return new ZipFilesystem(zip_file_, basedir_in_zip_file_ + "/" + localpath);
 }
 /**
  * Remove a number of files
@@ -243,7 +297,7 @@
 	throw ZipOperationError
 		("ZipFilesystem::unlink",
 		 filename,
-		 m_zipfilename,
+		 zip_file_->path(),
 		 "unlinking is not supported inside zipfiles");
 }
 
@@ -267,8 +321,6 @@
  * if either ondir or otherdir is missing
  */
 void ZipFilesystem::make_directory(const std::string & dirname) {
-	m_open_zip();
-
 	zip_fileinfo zi;
 
 	zi.tmz_date.tm_sec = zi.tmz_date.tm_min = zi.tmz_date.tm_hour =
@@ -277,7 +329,7 @@
 	zi.internal_fa = 0;
 	zi.external_fa = 0;
 
-	std::string complete_filename = m_basename;
+	std::string complete_filename = basedir_in_zip_file_;
 	complete_filename += "/";
 	complete_filename += dirname;
 
@@ -285,21 +337,9 @@
 	if (*complete_filename.rbegin() != '/')
 		complete_filename += '/';
 
-	switch
-		(zipOpenNewFileInZip3
-		 	(m_zipfile,
-			 complete_filename.c_str(),
-		 	 &zi,
-		 	 nullptr, 0, nullptr, 0, nullptr /* comment*/,
-		 	 Z_DEFLATED,
-		 	 Z_BEST_COMPRESSION,
-		 	 0,
-		 	 -MAX_WBITS,
-		 	 DEF_MEM_LEVEL,
-		 	 Z_DEFAULT_STRATEGY,
-		 	 nullptr,
-		 	 0))
-	{
+	switch (zipOpenNewFileInZip3(zip_file_->write_handle(), complete_filename.c_str(), &zi, nullptr,
+	                             0, nullptr, 0, nullptr /* comment*/, Z_DEFLATED, Z_BEST_COMPRESSION,
+	                             0, -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY, nullptr, 0)) {
 	case ZIP_OK:
 		break;
 	case ZIP_ERRNO:
@@ -309,8 +349,7 @@
 		throw FileError
 			("ZipFilesystem::make_directory", complete_filename);
 	}
-
-	zipCloseFileInZip(m_zipfile);
+	zipCloseFileInZip(zip_file_->write_handle());
 }
 
 /**
@@ -322,34 +361,34 @@
 		throw ZipOperationError
 			("ZipFilesystem::load",
 			 fname,
-			 m_zipfilename,
+			 zip_file_->path(),
 			 "could not open file from zipfile");
 
 	char buffer[1024];
 	size_t totallen = 0;
-	unzOpenCurrentFile(m_unzipfile);
+	unzOpenCurrentFile(zip_file_->read_handle());
 	for (;;) {
 		const int32_t len =
-			unzReadCurrentFile(m_unzipfile, buffer, sizeof(buffer));
+			unzReadCurrentFile(zip_file_->read_handle(), buffer, sizeof(buffer));
 		if (len == 0)
 			break;
 		if (len < 0) {
-			unzCloseCurrentFile(m_unzipfile);
+			unzCloseCurrentFile(zip_file_->read_handle());
 			const std::string errormessage = (boost::format("read error %i") % len).str();
 			throw ZipOperationError
-				("ZipFilesystem::load", fname, m_zipfilename, errormessage.c_str());
+				("ZipFilesystem::load", fname, zip_file_->path(), errormessage.c_str());
 		}
 
 		totallen += len;
 	}
-	unzCloseCurrentFile(m_unzipfile);
+	unzCloseCurrentFile(zip_file_->read_handle());
 
 	void * const result = malloc(totallen + 1);
 	if (!result)
 		throw std::bad_alloc();
-	unzOpenCurrentFile(m_unzipfile);
-	unzReadCurrentFile(m_unzipfile, result, totallen);
-	unzCloseCurrentFile(m_unzipfile);
+	unzOpenCurrentFile(zip_file_->read_handle());
+	unzReadCurrentFile(zip_file_->read_handle(), result, totallen);
+	unzCloseCurrentFile(zip_file_->read_handle());
 
 	static_cast<uint8_t *>(result)[totallen] = 0;
 	length = totallen;
@@ -367,36 +406,27 @@
 	std::string filename = fname;
 	std::replace(filename.begin(), filename.end(), '\\', '/');
 
-	m_open_zip();
-
 	zip_fileinfo zi;
-
 	zi.tmz_date.tm_sec = zi.tmz_date.tm_min = zi.tmz_date.tm_hour =
 		zi.tmz_date.tm_mday = zi.tmz_date.tm_mon = zi.tmz_date.tm_year = 0;
 	zi.dosDate     = 0;
 	zi.internal_fa = 0;
 	zi.external_fa = 0;
 
-	std::string complete_filename = m_basename + "/" + filename;
+	std::string complete_filename = basedir_in_zip_file_ + "/" + filename;
 
 	//  create file
-	switch
-		(zipOpenNewFileInZip3
-			(m_zipfile, complete_filename.c_str(), &zi,
-		 	 nullptr, 0, nullptr, 0, nullptr /* comment*/,
-		 	 Z_DEFLATED,
-		 	 Z_BEST_COMPRESSION, 0,
-		 	 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
-		 	 nullptr, 0))
-	{
+	switch (zipOpenNewFileInZip3(zip_file_->write_handle(), complete_filename.c_str(), &zi, nullptr,
+	                             0, nullptr, 0, nullptr /* comment*/, Z_DEFLATED, Z_BEST_COMPRESSION,
+	                             0, -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY, nullptr, 0)) {
 	case ZIP_OK:
 		break;
 	default:
-		throw ZipOperationError
-			("ZipFilesystem::write", complete_filename, m_zipfilename);
+		throw ZipOperationError(
+		   "ZipFilesystem::write", complete_filename, zip_file_->path());
 	}
 
-	switch (zipWriteInFileInZip (m_zipfile, data, length)) {
+	switch (zipWriteInFileInZip (zip_file_->write_handle(), data, length)) {
 	case ZIP_OK:
 		break;
 	case ZIP_ERRNO:
@@ -407,22 +437,80 @@
 			("ZipFilesystem::write", complete_filename);
 	}
 
-	zipCloseFileInZip(m_zipfile);
-}
-
-//
-// Stream Read
-//
-
-ZipFilesystem::ZipStreamRead::ZipStreamRead(zipFile file, ZipFilesystem* zipfs)
-: m_unzipfile(file), m_zipfs(zipfs) {}
+	zipCloseFileInZip(zip_file_->write_handle());
+}
+
+StreamRead* ZipFilesystem::open_stream_read(const std::string& fname) {
+	if (!file_exists(fname.c_str()) || is_directory(fname.c_str()))
+		throw ZipOperationError
+			("ZipFilesystem::load",
+			 fname,
+			 zip_file_->path(),
+			 "could not open file from zipfile");
+
+	int32_t method;
+	int result = unzOpenCurrentFile3(zip_file_->read_handle(), &method, nullptr, 1, nullptr);
+	switch (result) {
+		case ZIP_OK:
+			break;
+		default:
+			throw ZipOperationError
+				("ZipFilesystem: Failed to open streamwrite", fname, zip_file_->path());
+	}
+	return new ZipStreamRead(zip_file_);
+}
+
+StreamWrite * ZipFilesystem::open_stream_write(const std::string & fname) {
+	zip_fileinfo zi;
+
+	zi.tmz_date.tm_sec = zi.tmz_date.tm_min = zi.tmz_date.tm_hour =
+		zi.tmz_date.tm_mday = zi.tmz_date.tm_mon = zi.tmz_date.tm_year = 0;
+	zi.dosDate     = 0;
+	zi.internal_fa = 0;
+	zi.external_fa = 0;
+
+	std::string complete_filename = basedir_in_zip_file_ + "/" + fname;
+	//  create file
+	switch
+		(zipOpenNewFileInZip3
+			(zip_file_->write_handle(), complete_filename.c_str(), &zi,
+		 	 nullptr, 0, nullptr, 0, nullptr /* comment*/,
+		 	 Z_DEFLATED,
+		 	 Z_BEST_COMPRESSION, 0,
+		 	 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
+		 	 nullptr, 0))
+	{
+	case ZIP_OK:
+		break;
+	default:
+		throw ZipOperationError
+			("ZipFilesystem: Failed to open streamwrite", complete_filename, zip_file_->path());
+	}
+	return new ZipStreamWrite(zip_file_);
+}
+
+void ZipFilesystem::fs_rename(const std::string &, const std::string &) {
+	throw wexception("rename inside zip FS is not implemented yet");
+}
+
+unsigned long long ZipFilesystem::disk_space() {
+	return 0;
+}
+
+std::string ZipFilesystem::get_basename() {
+	return zip_file_->path();
+}
+
+ZipFilesystem::ZipStreamRead::ZipStreamRead(const std::shared_ptr<ZipFile>& shared_data)
+   : zip_file_(shared_data) {
+}
+
 ZipFilesystem::ZipStreamRead::~ZipStreamRead() {
-	m_zipfs->m_close();
 }
 
 size_t ZipFilesystem::ZipStreamRead::data(void* read_data, size_t bufsize)
 {
-	int copied = unzReadCurrentFile(m_unzipfile, read_data, bufsize);
+	int copied = unzReadCurrentFile(zip_file_->read_handle(), read_data, bufsize);
 	if (copied < 0) {
 		throw new DataError("Failed to read from zip file");
 	}
@@ -431,46 +519,22 @@
 	}
 	return copied;
 }
+
 bool ZipFilesystem::ZipStreamRead::end_of_file() const
 {
-	return unzReadCurrentFile(m_unzipfile, nullptr, 1) == 0;
-}
-
-StreamRead* ZipFilesystem::open_stream_read(const std::string& fname) {
-	if (!file_exists(fname.c_str()) || is_directory(fname.c_str()))
-		throw ZipOperationError
-			("ZipFilesystem::load",
-			 fname,
-			 m_zipfilename,
-			 "could not open file from zipfile");
-
-	int32_t method;
-	m_open_unzip();
-	int result = unzOpenCurrentFile3(m_unzipfile, &method, nullptr, 1, nullptr);
-	switch (result) {
-		case ZIP_OK:
-			break;
-		default:
-			throw ZipOperationError
-				("ZipFilesystem: Failed to open streamwrite", fname, m_zipfilename);
-	}
-	return new ZipStreamRead(m_unzipfile, this);
-}
-
-//
-// Stream write
-//
-
-ZipFilesystem::ZipStreamWrite::ZipStreamWrite(zipFile file, ZipFilesystem* zipfs)
-: m_zipfile(file), m_zipfs(zipfs) {}
-ZipFilesystem::ZipStreamWrite::~ZipStreamWrite()
-{
-	m_zipfs->m_close();
+	return unzReadCurrentFile(zip_file_->read_handle(), nullptr, 1) == 0;
+}
+
+ZipFilesystem::ZipStreamWrite::ZipStreamWrite(const std::shared_ptr<ZipFile>& shared_data)
+   : zip_file_(shared_data) {
+}
+
+ZipFilesystem::ZipStreamWrite::~ZipStreamWrite() {
 }
 
 void ZipFilesystem::ZipStreamWrite::data(const void* const write_data, const size_t size)
 {
-	int result = zipWriteInFileInZip(m_zipfile, write_data, size);
+	int result = zipWriteInFileInZip(zip_file_->write_handle(), write_data, size);
 	switch (result) {
 		case ZIP_OK:
 			break;
@@ -478,111 +542,3 @@
 			throw wexception("Failed to write into zipfile");
 	}
 }
-
-StreamWrite * ZipFilesystem::open_stream_write(const std::string & fname) {
-	m_open_zip();
-
-	zip_fileinfo zi;
-
-	zi.tmz_date.tm_sec = zi.tmz_date.tm_min = zi.tmz_date.tm_hour =
-		zi.tmz_date.tm_mday = zi.tmz_date.tm_mon = zi.tmz_date.tm_year = 0;
-	zi.dosDate     = 0;
-	zi.internal_fa = 0;
-	zi.external_fa = 0;
-
-	std::string complete_filename = m_basename + "/" + fname;
-	//  create file
-	switch
-		(zipOpenNewFileInZip3
-			(m_zipfile, complete_filename.c_str(), &zi,
-		 	 nullptr, 0, nullptr, 0, nullptr /* comment*/,
-		 	 Z_DEFLATED,
-		 	 Z_BEST_COMPRESSION, 0,
-		 	 -MAX_WBITS, DEF_MEM_LEVEL, Z_DEFAULT_STRATEGY,
-		 	 nullptr, 0))
-	{
-	case ZIP_OK:
-		break;
-	default:
-		throw ZipOperationError
-			("ZipFilesystem: Failed to open streamwrite", complete_filename, m_zipfilename);
-	}
-	return new ZipStreamWrite(m_zipfile, this);
-}
-
-
-/**
- * Private Functions below
- */
-void ZipFilesystem::m_close() {
-	if (m_state == STATE_ZIPPING)
-		zipClose(m_zipfile, nullptr);
-	else if (m_state == STATE_UNZIPPPING)
-		unzClose(m_unzipfile);
-
-	m_state = STATE_IDLE;
-}
-
-/**
- * Open a zipfile for compressing
- */
-void ZipFilesystem::m_open_zip() {
-	if (m_state == STATE_ZIPPING)
-		return;
-
-	m_close();
-
-	m_zipfile = zipOpen(m_zipfilename.c_str(), APPEND_STATUS_ADDINZIP);
-	if (!m_zipfile) {
-		//  Couldn't open for append, so create new.
-		m_zipfile = zipOpen(m_zipfilename.c_str(), APPEND_STATUS_CREATE);
-	}
-
-	m_state = STATE_ZIPPING;
-}
-
-/**
- * Open a zipfile for extraction
- * \throw FileTypeError
- */
-void ZipFilesystem::m_open_unzip() {
-	if (m_state == STATE_UNZIPPPING)
-		return;
-
-	m_close();
-
-	m_unzipfile = unzOpen(m_zipfilename.c_str());
-	if (!m_unzipfile)
-		throw FileTypeError
-			("ZipFilesystem::m_open_unzip", m_zipfilename, "not a .zip file");
-
-	m_state = STATE_UNZIPPPING;
-}
-
-void ZipFilesystem::fs_rename(const std::string &, const std::string &) {
-	throw wexception("rename inside zip FS is not implemented yet");
-}
-
-unsigned long long ZipFilesystem::disk_space() {
-	return 0;
-}
-
-std::string ZipFilesystem::strip_basename(std::string filename)
-{
-
-	if (filename.compare(0, m_basenamezip.length(), m_basenamezip) == 0)
-	{
-		// filename contains the name of the zip file as first element. This means
-		// this is an old zip file where all data were in a directory named as the
-		// file inside the zip file.
-		// return the filename without the first element
-		m_oldzip = true;
-		return filename.substr(m_basenamezip.length() + 1);
-	}
-
-	// seems to be a new zipfile without directory or a old zipfile was renamed
-	m_oldzip = false;
-	if (*filename.begin() == '/')
-		return filename.substr(1);
-	return filename;
-}

=== modified file 'src/io/filesystem/zip_filesystem.h'
--- src/io/filesystem/zip_filesystem.h	2014-09-29 13:30:46 +0000
+++ src/io/filesystem/zip_filesystem.h	2016-01-17 21:52:36 +0000
@@ -21,6 +21,7 @@
 #define WL_IO_FILESYSTEM_ZIP_FILESYSTEM_H
 
 #include <cstring>
+#include <memory>
 #include <string>
 
 #include "base/macros.h"
@@ -63,50 +64,87 @@
 
 	static FileSystem * create_from_directory(const std::string & directory);
 
-	std::string get_basename() override {return m_zipfilename;}
-
-protected:
-	void m_open_unzip();
-	void m_open_zip();
-	void m_close();
-	std::string strip_basename(std::string);
-
-	enum State {
-		STATE_IDLE,
-		STATE_ZIPPING,
-		STATE_UNZIPPPING
+	std::string get_basename() override;
+
+private:
+	enum class State {kIdle, kZipping, kUnzipping};
+
+	// All zip filesystems that use the same zip file have a shared
+	// state, which is represented in this struct. This is usually
+	// shared between the root file system, plus every filesystem generated
+	// through 'make_sub_file_system'.
+	class ZipFile {
+	public:
+		ZipFile(const std::string& zipfile);
+
+		// Calls 'close()'.
+		~ZipFile();
+
+		// Make 'filename' into a relative part, dealing with legacy Widelands
+		// zip file format.
+		std::string strip_basename(const std::string& filename);
+
+		// Full path to the zip file.
+		const std::string& path() const;
+
+		// Closes the file if it is open, reopens it for writing, and
+		// returns the minizip handle.
+		const zipFile& write_handle();
+
+		// Closes the file if it is open, reopens it for reading, and returns the
+		// minizip handle.
+		const unzFile& read_handle();
+
+	private:
+		// Closes 'path_' and reopens it for unzipping (read).
+		void open_for_unzip();
+
+		// Closes 'path_' and reopens it for zipping (write).
+		void open_for_zip();
+
+		// Closes 'path_' if it is opened.
+		void close();
+
+		State state_;
+
+		// E.g. "path/to/filename.zip"
+		std::string path_;
+
+		// E.g. "filename.zip"
+		std::string basename_;
+
+		// File handles for zipping and unzipping.
+		zipFile write_handle_;
+		unzFile read_handle_;
 	};
 
-	State       m_state;
-	zipFile     m_zipfile;
-	unzFile     m_unzipfile;
-	/// if true data is in a directory named as the zipfile. This is set by
-	/// strip_basename()
-	bool        m_oldzip;
-	std::string m_zipfilename;
-	std::string m_basenamezip;
-	std::string m_basename;
-
 	struct ZipStreamRead : StreamRead {
-		explicit ZipStreamRead(zipFile file, ZipFilesystem* zipfs);
+		explicit ZipStreamRead(const std::shared_ptr<ZipFile>& shared_data);
 		virtual ~ZipStreamRead();
 		size_t data(void* data, size_t bufsize) override;
 		bool end_of_file() const override;
 	private:
-		zipFile m_unzipfile;
-		ZipFilesystem* m_zipfs;
+		std::shared_ptr<ZipFile> zip_file_;
 	};
+
 	struct ZipStreamWrite : StreamWrite {
-		explicit ZipStreamWrite(zipFile file, ZipFilesystem* zipfs);
+		explicit ZipStreamWrite(const std::shared_ptr<ZipFile>& shared_data);
 		virtual ~ZipStreamWrite();
 		void data(const void* const data, size_t size) override;
 	private:
-		zipFile m_zipfile;
-		ZipFilesystem* m_zipfs;
+		std::shared_ptr<ZipFile> zip_file_;
 	};
 
+	// Used for creating sub filesystems.
+	ZipFilesystem(const std::shared_ptr<ZipFile>& shared_data, const std::string& basedir_in_zip_file);
+
+	// The data shared between all zip filesystems with the same
+	// underlying zip file.
+	std::shared_ptr<ZipFile> zip_file_;
+
+	// If we are a sub filesystem, this points to our base
+	// directory, otherwise it is the empty string.
+	std::string basedir_in_zip_file_;
 };
 
-
-
 #endif  // end of include guard: WL_IO_FILESYSTEM_ZIP_FILESYSTEM_H