[Scummvm-git-logs] scummvm master -> 99dc7641a998cc26fa2aa6eaf0c55f22f4722e33

sev- noreply at scummvm.org
Tue Oct 1 20:43:42 UTC 2024


This automated email contains information about 2 new commits which have been
pushed to the 'scummvm' repo located at https://github.com/scummvm/scummvm .

Summary:
5d0d6b7379 BACKENDS: FS: Refactor StdioStream::makeFromPath
99dc7641a9 BACKENDS: FS: Make file writing atomic


Commit: 5d0d6b7379714ceff94fe68c4a8489e7ce3782e1
    https://github.com/scummvm/scummvm/commit/5d0d6b7379714ceff94fe68c4a8489e7ce3782e1
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-10-01T22:43:38+02:00

Commit Message:
BACKENDS: FS: Refactor StdioStream::makeFromPath

This allows to avoid code duplication with PosixIoStream

Changed paths:
    backends/fs/posix-drives/posix-drives-fs.cpp
    backends/fs/posix/posix-iostream.cpp
    backends/fs/posix/posix-iostream.h
    backends/fs/stdiostream.cpp
    backends/fs/stdiostream.h


diff --git a/backends/fs/posix-drives/posix-drives-fs.cpp b/backends/fs/posix-drives/posix-drives-fs.cpp
index 861ecb1d3c6..df3b8fcbafe 100644
--- a/backends/fs/posix-drives/posix-drives-fs.cpp
+++ b/backends/fs/posix-drives/posix-drives-fs.cpp
@@ -69,7 +69,7 @@ void DrivePOSIXFilesystemNode::configureStream(StdioStream *stream) {
 }
 
 Common::SeekableReadStream *DrivePOSIXFilesystemNode::createReadStream() {
-	PosixIoStream *readStream = PosixIoStream::makeFromPath(getPath(), false);
+	StdioStream *readStream = PosixIoStream::makeFromPath(getPath(), false);
 
 	configureStream(readStream);
 
@@ -82,7 +82,7 @@ Common::SeekableReadStream *DrivePOSIXFilesystemNode::createReadStream() {
 }
 
 Common::SeekableWriteStream *DrivePOSIXFilesystemNode::createWriteStream() {
-	PosixIoStream *writeStream = PosixIoStream::makeFromPath(getPath(), true);
+	StdioStream *writeStream = PosixIoStream::makeFromPath(getPath(), true);
 
 	configureStream(writeStream);
 
diff --git a/backends/fs/posix/posix-iostream.cpp b/backends/fs/posix/posix-iostream.cpp
index bf70c0ecdbd..699f366626b 100644
--- a/backends/fs/posix/posix-iostream.cpp
+++ b/backends/fs/posix/posix-iostream.cpp
@@ -25,20 +25,6 @@
 
 #include <sys/stat.h>
 
-PosixIoStream *PosixIoStream::makeFromPath(const Common::String &path, bool writeMode) {
-#if defined(HAS_FOPEN64)
-	FILE *handle = fopen64(path.c_str(), writeMode ? "wb" : "rb");
-#else
-	FILE *handle = fopen(path.c_str(), writeMode ? "wb" : "rb");
-#endif
-
-	if (handle)
-		return new PosixIoStream(handle);
-
-	return nullptr;
-}
-
-
 PosixIoStream::PosixIoStream(void *handle) :
 		StdioStream(handle) {
 }
diff --git a/backends/fs/posix/posix-iostream.h b/backends/fs/posix/posix-iostream.h
index 741910d8c67..1f16ae44454 100644
--- a/backends/fs/posix/posix-iostream.h
+++ b/backends/fs/posix/posix-iostream.h
@@ -29,7 +29,11 @@
  */
 class PosixIoStream final : public StdioStream {
 public:
-	static PosixIoStream *makeFromPath(const Common::String &path, bool writeMode);
+	static StdioStream *makeFromPath(const Common::String &path, bool writeMode) {
+		return StdioStream::makeFromPathHelper(path, writeMode, [](void *handle) -> StdioStream * {
+			return new PosixIoStream(handle);
+		});
+	}
 	PosixIoStream(void *handle);
 
 	int64 size() const override;
diff --git a/backends/fs/stdiostream.cpp b/backends/fs/stdiostream.cpp
index 1f76cada6b4..f6b58e23ff5 100644
--- a/backends/fs/stdiostream.cpp
+++ b/backends/fs/stdiostream.cpp
@@ -124,7 +124,8 @@ bool StdioStream::flush() {
 	return fflush((FILE *)_handle) == 0;
 }
 
-StdioStream *StdioStream::makeFromPath(const Common::String &path, bool writeMode) {
+StdioStream *StdioStream::makeFromPathHelper(const Common::String &path, bool writeMode,
+		StdioStream *(*factory)(void *handle)) {
 #if defined(WIN32) && defined(UNICODE)
 	wchar_t *wPath = Win32::stringToTchar(path);
 	FILE *handle = _wfopen(wPath, writeMode ? L"wb" : L"rb");
@@ -136,7 +137,7 @@ StdioStream *StdioStream::makeFromPath(const Common::String &path, bool writeMod
 #endif
 
 	if (handle)
-		return new StdioStream(handle);
+		return new factory(handle);
 	return nullptr;
 }
 
diff --git a/backends/fs/stdiostream.h b/backends/fs/stdiostream.h
index a26fe2590a4..c6856260b88 100644
--- a/backends/fs/stdiostream.h
+++ b/backends/fs/stdiostream.h
@@ -32,12 +32,19 @@ protected:
 	/** File handle to the actual file. */
 	void *_handle;
 
+	static StdioStream *makeFromPathHelper(const Common::String &path, bool writeMode,
+			StdioStream *(*factory)(void *handle));
+
 public:
 	/**
 	 * Given a path, invokes fopen on that path and wrap the result in a
 	 * StdioStream instance.
 	 */
-	static StdioStream *makeFromPath(const Common::String &path, bool writeMode);
+	static StdioStream *makeFromPath(const Common::String &path, bool writeMode) {
+		return makeFromPathHelper(path, writeMode, [](void *handle) {
+			return new StdioStream(handle);
+		});
+	}
 
 	StdioStream(void *handle);
 	~StdioStream() override;


Commit: 99dc7641a998cc26fa2aa6eaf0c55f22f4722e33
    https://github.com/scummvm/scummvm/commit/99dc7641a998cc26fa2aa6eaf0c55f22f4722e33
Author: Le Philousophe (lephilousophe at users.noreply.github.com)
Date: 2024-10-01T22:43:38+02:00

Commit Message:
BACKENDS: FS: Make file writing atomic

A temporary file is used to write the data and the file is renamed when
it is closed.
The string containing the destination path is a stored as a pointer to
limit the size of the object (which is already doubled).

Changed paths:
    backends/fs/stdiostream.cpp
    backends/fs/stdiostream.h
    backends/log/log.cpp


diff --git a/backends/fs/stdiostream.cpp b/backends/fs/stdiostream.cpp
index f6b58e23ff5..5c5c5a26ea7 100644
--- a/backends/fs/stdiostream.cpp
+++ b/backends/fs/stdiostream.cpp
@@ -33,13 +33,26 @@
 
 // Include this after windows.h so we don't get a warning for redefining ARRAYSIZE
 #include "backends/fs/stdiostream.h"
+#include "common/textconsole.h"
 
-StdioStream::StdioStream(void *handle) : _handle(handle) {
+StdioStream::StdioStream(void *handle) : _handle(handle), _path(nullptr) {
 	assert(handle);
 }
 
 StdioStream::~StdioStream() {
 	fclose((FILE *)_handle);
+
+	if (!_path) {
+		return;
+	}
+
+	Common::String tmpPath(*_path);
+	tmpPath += ".tmp";
+	if (rename(tmpPath.c_str(), _path->c_str())) {
+		warning("Couldn't save file %s", _path->c_str());
+	}
+
+	delete _path;
 }
 
 bool StdioStream::err() const {
@@ -126,19 +139,30 @@ bool StdioStream::flush() {
 
 StdioStream *StdioStream::makeFromPathHelper(const Common::String &path, bool writeMode,
 		StdioStream *(*factory)(void *handle)) {
+	Common::String tmpPath(path);
+	if (writeMode) {
+		tmpPath += ".tmp";
+	}
 #if defined(WIN32) && defined(UNICODE)
-	wchar_t *wPath = Win32::stringToTchar(path);
+	wchar_t *wPath = Win32::stringToTchar(tmpPath);
 	FILE *handle = _wfopen(wPath, writeMode ? L"wb" : L"rb");
 	free(wPath);
 #elif defined(HAS_FOPEN64)
-	FILE *handle = fopen64(path.c_str(), writeMode ? "wb" : "rb");
+	FILE *handle = fopen64(tmpPath.c_str(), writeMode ? "wb" : "rb");
 #else
-	FILE *handle = fopen(path.c_str(), writeMode ? "wb" : "rb");
+	FILE *handle = fopen(tmpPath.c_str(), writeMode ? "wb" : "rb");
 #endif
 
-	if (handle)
-		return new factory(handle);
-	return nullptr;
+	if (!handle) {
+		return nullptr;
+	}
+
+	StdioStream *stream = factory(handle);
+	if (writeMode) {
+		stream->_path = new Common::String(path);
+	}
+
+	return stream;
 }
 
 #endif
diff --git a/backends/fs/stdiostream.h b/backends/fs/stdiostream.h
index c6856260b88..ef17c5cc0fe 100644
--- a/backends/fs/stdiostream.h
+++ b/backends/fs/stdiostream.h
@@ -31,6 +31,7 @@ class StdioStream : public Common::SeekableReadStream, public Common::SeekableWr
 protected:
 	/** File handle to the actual file. */
 	void *_handle;
+	Common::String *_path;
 
 	static StdioStream *makeFromPathHelper(const Common::String &path, bool writeMode,
 			StdioStream *(*factory)(void *handle));
diff --git a/backends/log/log.cpp b/backends/log/log.cpp
index dc98dcb2f17..44271232b89 100644
--- a/backends/log/log.cpp
+++ b/backends/log/log.cpp
@@ -56,8 +56,11 @@ void Log::close() {
 		// Output a message to indicate that the log was closed successfully
 		print("--- Log closed successfully.\n");
 
-		delete _stream;
+		// This avoids a segfault if a warning is issued when deleting the stream
+		Common::WriteStream *stream = _stream;
 		_stream = nullptr;
+
+		delete stream;
 	}
 }
 




More information about the Scummvm-git-logs mailing list