core: do not assert() but wrap and return error codes instead

src/core/filesystem/filesystem.h

@@ -45,13 +45,13 @@ struct Filesystem
 	virtual u64 last_modified_time(const char* path) = 0;
 
 	/// Creates the directory at the given @a path.
-	virtual void create_directory(const char* path) = 0;
+	virtual CreateResult create_directory(const char* path) = 0;
 
 	/// Deletes the directory at the given @a path.
-	virtual void delete_directory(const char* path) = 0;
+	virtual DeleteResult delete_directory(const char* path) = 0;
 
 	/// Deletes the file at the given @a path.
-	virtual void delete_file(const char* path) = 0;
+	virtual DeleteResult delete_file(const char* path) = 0;
 
 	/// Returns the relative file names in the given @a path.
 	virtual void list_files(const char* path, Vector<DynamicString>& files) = 0;

src/core/filesystem/filesystem_apk.cpp

@@ -162,19 +162,25 @@ u64 FilesystemApk::last_modified_time(const char* path)
 	return 0;
 }
 
-void FilesystemApk::create_directory(const char* /*path*/)
+CreateResult FilesystemApk::create_directory(const char* /*path*/)
 {
 	CE_FATAL("Cannot create directory in Android assets folder");
+	CreateResult cr.error = CreateResult::UNKNOWN;
+	return cr;
 }
 
-void FilesystemApk::delete_directory(const char* /*path*/)
+DeleteResult FilesystemApk::delete_directory(const char* /*path*/)
 {
 	CE_FATAL("Cannot delete directory in Android assets folder");
+	DeleteResult dr.error = DeleteResult::UNKNOWN;
+	return dr;
 }
 
-void FilesystemApk::delete_file(const char* /*path*/)
+DeleteResult FilesystemApk::delete_file(const char* /*path*/)
 {
 	CE_FATAL("Cannot delete file in Android assets folder");
+	DeleteResult dr.error = DeleteResult::UNKNOWN;
+	return dr;
 }
 
 void FilesystemApk::list_files(const char* path, Vector<DynamicString>& files)

src/core/filesystem/filesystem_apk.h

@@ -48,13 +48,13 @@ struct FilesystemApk : public Filesystem
 	u64 last_modified_time(const char* path);
 
 	/// @copydoc Filesystem::create_directory()
-	void create_directory(const char* path);
+	CreateResult create_directory(const char* path);
 
 	/// @copydoc Filesystem::delete_directory()
-	void delete_directory(const char* path);
+	DeleteResult delete_directory(const char* path);
 
 	/// @copydoc Filesystem::delete_file()
-	void delete_file(const char* path);
+	DeleteResult delete_file(const char* path);
 
 	/// @copydoc Filesystem::list_files()
 	void list_files(const char* path, Vector<DynamicString>& files);

src/core/filesystem/filesystem_disk.cpp

@@ -287,7 +287,7 @@ u64 FilesystemDisk::last_modified_time(const char* path)
 	return stat(path).mtime;
 }
 
-void FilesystemDisk::create_directory(const char* path)
+CreateResult FilesystemDisk::create_directory(const char* path)
 {
 	CE_ENSURE(NULL != path);
 
@@ -295,15 +295,10 @@ void FilesystemDisk::create_directory(const char* path)
 	DynamicString abs_path(ta);
 	get_absolute_path(path, abs_path);
 
-	Stat info;
-	os::stat(info, abs_path.c_str());
-	if (info.file_type != Stat::NO_ENTRY)
-		return;
-
-	os::create_directory(abs_path.c_str());
+	return os::create_directory(abs_path.c_str());
 }
 
-void FilesystemDisk::delete_directory(const char* path)
+DeleteResult FilesystemDisk::delete_directory(const char* path)
 {
 	CE_ENSURE(NULL != path);
 
@@ -311,10 +306,10 @@ void FilesystemDisk::delete_directory(const char* path)
 	DynamicString abs_path(ta);
 	get_absolute_path(path, abs_path);
 
-	os::delete_directory(abs_path.c_str());
+	return os::delete_directory(abs_path.c_str());
 }
 
-void FilesystemDisk::delete_file(const char* path)
+DeleteResult FilesystemDisk::delete_file(const char* path)
 {
 	CE_ENSURE(NULL != path);
 
@@ -322,7 +317,7 @@ void FilesystemDisk::delete_file(const char* path)
 	DynamicString abs_path(ta);
 	get_absolute_path(path, abs_path);
 
-	os::delete_file(abs_path.c_str());
+	return os::delete_file(abs_path.c_str());
 }
 
 void FilesystemDisk::list_files(const char* path, Vector<DynamicString>& files)

src/core/filesystem/filesystem_disk.h

@@ -52,13 +52,13 @@ struct FilesystemDisk : public Filesystem
 	u64 last_modified_time(const char* path);
 
 	/// @copydoc Filesystem::create_directory()
-	void create_directory(const char* path);
+	CreateResult create_directory(const char* path);
 
 	/// @copydoc Filesystem::delete_directory()
-	void delete_directory(const char* path);
+	DeleteResult delete_directory(const char* path);
 
 	/// @copydoc Filesystem::delete_file()
-	void delete_file(const char* path);
+	DeleteResult delete_file(const char* path);
 
 	/// @copydoc Filesystem::list_files()
 	void list_files(const char* path, Vector<DynamicString>& files);

src/core/os.cpp

@@ -140,43 +140,71 @@ namespace os
 		info.mtime = buf.st_mtime;
 	}
 
-	void delete_file(const char* path)
+	DeleteResult delete_file(const char* path)
 	{
+		DeleteResult dr;
 #if CROWN_PLATFORM_POSIX
-		int err = ::unlink(path);
-		CE_ASSERT(err == 0, "unlink: errno = %d", errno);
-		CE_UNUSED(err);
+		if (::unlink(path) == 0)
+			dr.error = DeleteResult::SUCCESS;
+		else if (errno == ENOENT)
+			dr.error = DeleteResult::NO_ENTRY;
+		else
+			dr.error = DeleteResult::UNKNOWN;
 #elif CROWN_PLATFORM_WINDOWS
-		BOOL err = DeleteFile(path);
-		CE_ASSERT(err != 0, "DeleteFile: GetLastError = %d", GetLastError());
-		CE_UNUSED(err);
+		if (DeleteFile(path) != 0)
+			dr.error = DeleteResult::SUCCESS;
+		else if (GetLastError() == ERROR_FILE_NOT_FOUND)
+			dr.error = DeleteResult::NO_ENTRY;
+		// else if (GetLastError() == ERROR_ACCESS_DENIED)
+		// 	dr.error = DeleteResult::NOT_FILE;
+		else
+			dr.error = DeleteResult::UNKNOWN;
 #endif
+		return dr;
 	}
 
-	void create_directory(const char* path)
+	CreateResult create_directory(const char* path)
 	{
+		CreateResult cr;
 #if CROWN_PLATFORM_POSIX
-		int err = ::mkdir(path, 0755);
-		CE_ASSERT(err == 0, "mkdir: errno = %d", errno);
-		CE_UNUSED(err);
+		if (::mkdir(path, 0755) == 0)
+			cr.error = CreateResult::SUCCESS;
+		else if (errno == EEXIST)
+			cr.error = CreateResult::EXISTS;
+		else
+			cr.error = CreateResult::UNKNOWN;
 #elif CROWN_PLATFORM_WINDOWS
-		BOOL err = CreateDirectory(path, NULL);
-		CE_ASSERT(err != 0, "CreateDirectory: GetLastError = %d", GetLastError());
-		CE_UNUSED(err);
+		if (CreateDirectory(path, NULL) != 0)
+			cr.error = CreateResult::SUCCESS;
+		else if (GetLastError() == ERROR_ALREADY_EXISTS)
+			cr.error = CreateResult::EXISTS;
+		else
+			cr.error = CreateResult::UNKNOWN;
 #endif
+		return cr;
 	}
 
-	void delete_directory(const char* path)
+	DeleteResult delete_directory(const char* path)
 	{
+		DeleteResult dr;
 #if CROWN_PLATFORM_POSIX
-		int err = ::rmdir(path);
-		CE_ASSERT(err == 0, "rmdir: errno = %d", errno);
-		CE_UNUSED(err);
+		if (::rmdir(path) == 0)
+			dr.error = DeleteResult::SUCCESS;
+		else if (errno == ENOENT)
+			dr.error = DeleteResult::NO_ENTRY;
+		else
+			dr.error = DeleteResult::UNKNOWN;
 #elif CROWN_PLATFORM_WINDOWS
-		BOOL err = RemoveDirectory(path);
-		CE_ASSERT(err != 0, "RemoveDirectory: GetLastError = %d", GetLastError());
-		CE_UNUSED(err);
+		if (RemoveDirectory(path) != 0)
+			dr.error = DeleteResult::SUCCESS;
+		else if (GetLastError() == ERROR_FILE_NOT_FOUND)
+			dr.error = DeleteResult::NO_ENTRY;
+		// else if (GetLastError() == ERROR_DIRECTORY
+		// 	dr.error = DeleteResult::NOT_DIRECTORY;
+		else
+			dr.error = DeleteResult::UNKNOWN;
 #endif
+		return dr;
 	}
 
 	const char* getcwd(char* buf, u32 size)

src/core/os.h

@@ -28,6 +28,32 @@ struct Stat
 	u64 mtime; ///< Last modified time.
 };
 
+/// Result from os::delete_file() or os::delete_directory().
+///
+/// @a ingroup OS
+struct DeleteResult
+{
+	enum
+	{
+		SUCCESS,  ///<
+		NO_ENTRY, ///< Entry does not exist.
+		UNKNOWN   ///< Unknown error.
+	} error;
+};
+
+/// Result from os::create_directory().
+///
+/// @a ingroup OS
+struct CreateResult
+{
+	enum
+	{
+		SUCCESS,        ///<
+		ALREADY_EXISTS, ///< The entry already exists.
+		UNKNOWN         ///< Unknown error.
+	} error;
+};
+
 /// File access flags to be used with os::access().
 ///
 /// @ingroup OS
@@ -71,13 +97,14 @@ namespace os
 	void stat(Stat& info, const char* path);
 
 	/// Deletes the file at @a path.
-	void delete_file(const char* path);
+	DeleteResult delete_file(const char* path);
 
-	/// Creates a directory named @a path.
-	void create_directory(const char* path);
+	/// Creates a directory @a path.
+	CreateResult create_directory(const char* path);
 
-	/// Deletes the directory at @a path.
-	void delete_directory(const char* path);
+	/// Deletes the directory @a path.
+	/// The directory must be empty.
+	DeleteResult delete_directory(const char* path);
 
 	/// Returns the list of @a files at the given @a path.
 	void list_files(const char* path, Vector<DynamicString>& files);

src/core/unit_tests.cpp

@@ -29,6 +29,7 @@
 #include "core/memory/memory.h"
 #include "core/memory/temp_allocator.h"
 #include "core/murmur.h"
+#include "core/os.h"
 #include "core/process.h"
 #include "core/strings/dynamic_string.h"
 #include "core/strings/string.h"
@@ -1381,6 +1382,25 @@ static void test_process()
 #endif
 }
 
+static void test_filesystem()
+{
+#if CROWN_PLATFORM_POSIX
+	{
+		DeleteResult dr = os::delete_directory("/tmp/none");
+		ENSURE(dr.error == DeleteResult::NO_ENTRY);
+	}
+	{
+		os::delete_directory("/tmp/crown");
+		CreateResult cr = os::create_directory("/tmp/crown");
+		ENSURE(cr.error == CreateResult::SUCCESS);
+		             cr = os::create_directory("/tmp/crown");
+		ENSURE(cr.error == CreateResult::EXISTS);
+		DeleteResult dr = os::delete_directory("/tmp/crown");
+		ENSURE(dr.error == DeleteResult::SUCCESS);
+	}
+#endif // CROWN_PLATFORM_POSIX
+}
+
 #define RUN_TEST(name)      \
 	do {                    \
 		printf(#name "\n"); \
@@ -1413,6 +1433,7 @@ int main_unit_tests()
 	RUN_TEST(test_command_line);
 	RUN_TEST(test_thread);
 	RUN_TEST(test_process);
+	RUN_TEST(test_filesystem);
 
 	return EXIT_SUCCESS;
 }