Merge pull request #2132 from wwylele/fix-fs-err

Correct FS error codes & add path boundary checks

5 files changed, 37 insertions, 43 deletions

diff --git a/src/core/hle/result.h b/src/core/hle/result.h
index 7f8d8e00d..f7356f9d8 100644
--- a/src/core/hle/result.h
+++ b/src/core/hle/result.h
@@ -20,15 +20,24 @@ enum class ErrorDescription : u32 {
     OS_InvalidBufferDescriptor = 48,
     WrongAddress = 53,
     FS_ArchiveNotMounted = 101,
+    FS_FileNotFound = 112,
+    FS_PathNotFound = 113,
     FS_NotFound = 120,
+    FS_FileAlreadyExists = 180,
+    FS_DirectoryAlreadyExists = 185,
     FS_AlreadyExists = 190,
     FS_InvalidOpenFlags = 230,
+    FS_DirectoryNotEmpty = 240,
     FS_NotAFile = 250,
     FS_NotFormatted = 340, ///< This is used by the FS service when creating a SaveData archive
     OutofRangeOrMisalignedAddress =
         513, // TODO(purpasmart): Check if this name fits its actual usage
     GPU_FirstInitialization = 519,
+    FS_InvalidReadFlag = 700,
     FS_InvalidPath = 702,
+    FS_WriteBeyondEnd = 705,
+    FS_UnsupportedOpenFlags = 760,
+    FS_UnexpectedFileOrDirectory = 770,
     InvalidSection = 1000,
     TooLarge = 1001,
     NotAuthorized = 1002,
diff --git a/src/core/hle/service/cfg/cfg.cpp b/src/core/hle/service/cfg/cfg.cpp
index d3d0f3b55..d554c3f54 100644
--- a/src/core/hle/service/cfg/cfg.cpp
+++ b/src/core/hle/service/cfg/cfg.cpp
@@ -360,7 +360,7 @@ ResultCode CreateConfigInfoBlk(u32 block_id, u16 size, u16 flags, const void* da
 }
 
 ResultCode DeleteConfigNANDSaveFile() {
-    FileSys::Path path("config");
+    FileSys::Path path("/config");
     return Service::FS::DeleteFileFromArchive(cfg_system_save_data_archive, path);
 }
 
@@ -369,7 +369,7 @@ ResultCode UpdateConfigNANDSavegame() {
     mode.write_flag.Assign(1);
     mode.create_flag.Assign(1);
 
-    FileSys::Path path("config");
+    FileSys::Path path("/config");
 
     auto config_result = Service::FS::OpenFileFromArchive(cfg_system_save_data_archive, path, mode);
     ASSERT_MSG(config_result.Succeeded(), "could not open file");
@@ -383,8 +383,9 @@ ResultCode UpdateConfigNANDSavegame() {
 ResultCode FormatConfig() {
     ResultCode res = DeleteConfigNANDSaveFile();
     // The delete command fails if the file doesn't exist, so we have to check that too
-    if (!res.IsSuccess() && res.description != ErrorDescription::FS_NotFound)
+    if (!res.IsSuccess() && res.description != ErrorDescription::FS_FileNotFound) {
         return res;
+    }
     // Delete the old data
     cfg_config_file_buffer.fill(0);
     // Create the header
@@ -510,7 +511,7 @@ ResultCode LoadConfigNANDSaveFile() {
 
     cfg_system_save_data_archive = *archive_result;
 
-    FileSys::Path config_path("config");
+    FileSys::Path config_path("/config");
     FileSys::Mode open_mode = {};
     open_mode.read_flag.Assign(1);
 
diff --git a/src/core/hle/service/fs/archive.cpp b/src/core/hle/service/fs/archive.cpp
index 7f9696bfb..4c29784e8 100644
--- a/src/core/hle/service/fs/archive.cpp
+++ b/src/core/hle/service/fs/archive.cpp
@@ -15,9 +15,10 @@
 #include "common/logging/log.h"
 #include "core/file_sys/archive_backend.h"
 #include "core/file_sys/archive_extsavedata.h"
+#include "core/file_sys/archive_ncch.h"
 #include "core/file_sys/archive_savedata.h"
-#include "core/file_sys/archive_savedatacheck.h"
 #include "core/file_sys/archive_sdmc.h"
+#include "core/file_sys/archive_sdmcwriteonly.h"
 #include "core/file_sys/archive_systemsavedata.h"
 #include "core/file_sys/directory_backend.h"
 #include "core/file_sys/file_backend.h"
@@ -338,17 +339,11 @@ ResultCode RenameFileBetweenArchives(ArchiveHandle src_archive_handle,
         return ERR_INVALID_ARCHIVE_HANDLE;
 
     if (src_archive == dest_archive) {
-        if (src_archive->RenameFile(src_path, dest_path))
-            return RESULT_SUCCESS;
+        return src_archive->RenameFile(src_path, dest_path);
     } else {
         // TODO: Implement renaming across archives
         return UnimplementedFunction(ErrorModule::FS);
     }
-
-    // TODO(yuriks): This code probably isn't right, it'll return a Status even if the file didn't
-    // exist or similar. Verify.
-    return ResultCode(ErrorDescription::NoData, ErrorModule::FS, // TODO: verify description
-                      ErrorSummary::NothingHappened, ErrorLevel::Status);
 }
 
 ResultCode DeleteDirectoryFromArchive(ArchiveHandle archive_handle, const FileSys::Path& path) {
@@ -356,10 +351,7 @@ ResultCode DeleteDirectoryFromArchive(ArchiveHandle archive_handle, const FileSy
     if (archive == nullptr)
         return ERR_INVALID_ARCHIVE_HANDLE;
 
-    if (archive->DeleteDirectory(path))
-        return RESULT_SUCCESS;
-    return ResultCode(ErrorDescription::NoData, ErrorModule::FS, // TODO: verify description
-                      ErrorSummary::Canceled, ErrorLevel::Status);
+    return archive->DeleteDirectory(path);
 }
 
 ResultCode DeleteDirectoryRecursivelyFromArchive(ArchiveHandle archive_handle,
@@ -368,10 +360,7 @@ ResultCode DeleteDirectoryRecursivelyFromArchive(ArchiveHandle archive_handle,
     if (archive == nullptr)
         return ERR_INVALID_ARCHIVE_HANDLE;
 
-    if (archive->DeleteDirectoryRecursively(path))
-        return RESULT_SUCCESS;
-    return ResultCode(ErrorDescription::NoData, ErrorModule::FS, // TODO: verify description
-                      ErrorSummary::Canceled, ErrorLevel::Status);
+    return archive->DeleteDirectoryRecursively(path);
 }
 
 ResultCode CreateFileInArchive(ArchiveHandle archive_handle, const FileSys::Path& path,
@@ -388,10 +377,7 @@ ResultCode CreateDirectoryFromArchive(ArchiveHandle archive_handle, const FileSy
     if (archive == nullptr)
         return ERR_INVALID_ARCHIVE_HANDLE;
 
-    if (archive->CreateDirectory(path))
-        return RESULT_SUCCESS;
-    return ResultCode(ErrorDescription::NoData, ErrorModule::FS, // TODO: verify description
-                      ErrorSummary::Canceled, ErrorLevel::Status);
+    return archive->CreateDirectory(path);
 }
 
 ResultCode RenameDirectoryBetweenArchives(ArchiveHandle src_archive_handle,
@@ -404,17 +390,11 @@ ResultCode RenameDirectoryBetweenArchives(ArchiveHandle src_archive_handle,
         return ERR_INVALID_ARCHIVE_HANDLE;
 
     if (src_archive == dest_archive) {
-        if (src_archive->RenameDirectory(src_path, dest_path))
-            return RESULT_SUCCESS;
+        return src_archive->RenameDirectory(src_path, dest_path);
     } else {
         // TODO: Implement renaming across archives
         return UnimplementedFunction(ErrorModule::FS);
     }
-
-    // TODO(yuriks): This code probably isn't right, it'll return a Status even if the file didn't
-    // exist or similar. Verify.
-    return ResultCode(ErrorDescription::NoData, ErrorModule::FS, // TODO: verify description
-                      ErrorSummary::NothingHappened, ErrorLevel::Status);
 }
 
 ResultVal<Kernel::SharedPtr<Directory>> OpenDirectoryFromArchive(ArchiveHandle archive_handle,
@@ -423,13 +403,11 @@ ResultVal<Kernel::SharedPtr<Directory>> OpenDirectoryFromArchive(ArchiveHandle a
     if (archive == nullptr)
         return ERR_INVALID_ARCHIVE_HANDLE;
 
-    std::unique_ptr<FileSys::DirectoryBackend> backend = archive->OpenDirectory(path);
-    if (backend == nullptr) {
-        return ResultCode(ErrorDescription::FS_NotFound, ErrorModule::FS, ErrorSummary::NotFound,
-                          ErrorLevel::Permanent);
-    }
+    auto backend = archive->OpenDirectory(path);
+    if (backend.Failed())
+        return backend.Code();
 
-    auto directory = Kernel::SharedPtr<Directory>(new Directory(std::move(backend), path));
+    auto directory = Kernel::SharedPtr<Directory>(new Directory(backend.MoveFrom(), path));
     return MakeResult<Kernel::SharedPtr<Directory>>(std::move(directory));
 }
 
@@ -549,6 +527,13 @@ void RegisterArchiveTypes() {
         LOG_ERROR(Service_FS, "Can't instantiate SDMC archive with path %s",
                   sdmc_directory.c_str());
 
+    auto sdmcwo_factory = std::make_unique<FileSys::ArchiveFactory_SDMCWriteOnly>(sdmc_directory);
+    if (sdmcwo_factory->Initialize())
+        RegisterArchiveType(std::move(sdmcwo_factory), ArchiveIdCode::SDMCWriteOnly);
+    else
+        LOG_ERROR(Service_FS, "Can't instantiate SDMCWriteOnly archive with path %s",
+                  sdmc_directory.c_str());
+
     // Create the SaveData archive
     auto savedata_factory = std::make_unique<FileSys::ArchiveFactory_SaveData>(sdmc_directory);
     RegisterArchiveType(std::move(savedata_factory), ArchiveIdCode::SaveData);
@@ -569,10 +554,9 @@ void RegisterArchiveTypes() {
         LOG_ERROR(Service_FS, "Can't instantiate SharedExtSaveData archive with path %s",
                   sharedextsavedata_factory->GetMountPoint().c_str());
 
-    // Create the SaveDataCheck archive, basically a small variation of the RomFS archive
-    auto savedatacheck_factory =
-        std::make_unique<FileSys::ArchiveFactory_SaveDataCheck>(nand_directory);
-    RegisterArchiveType(std::move(savedatacheck_factory), ArchiveIdCode::SaveDataCheck);
+    // Create the NCCH archive, basically a small variation of the RomFS archive
+    auto savedatacheck_factory = std::make_unique<FileSys::ArchiveFactory_NCCH>(nand_directory);
+    RegisterArchiveType(std::move(savedatacheck_factory), ArchiveIdCode::NCCH);
 
     auto systemsavedata_factory =
         std::make_unique<FileSys::ArchiveFactory_SystemSaveData>(nand_directory);
diff --git a/src/core/hle/service/fs/archive.h b/src/core/hle/service/fs/archive.h
index 41a76285c..21ed9717b 100644
--- a/src/core/hle/service/fs/archive.h
+++ b/src/core/hle/service/fs/archive.h
@@ -33,7 +33,7 @@ enum class ArchiveIdCode : u32 {
     SystemSaveData = 0x00000008,
     SDMC = 0x00000009,
     SDMCWriteOnly = 0x0000000A,
-    SaveDataCheck = 0x2345678A,
+    NCCH = 0x2345678A,
 };
 
 /// Media types for the archives
diff --git a/src/core/hle/service/ptm/ptm.cpp b/src/core/hle/service/ptm/ptm.cpp
index 6e6b63329..cc859c14c 100644
--- a/src/core/hle/service/ptm/ptm.cpp
+++ b/src/core/hle/service/ptm/ptm.cpp
@@ -128,7 +128,7 @@ void Init() {
             Service::FS::OpenArchive(Service::FS::ArchiveIdCode::SharedExtSaveData, archive_path);
         ASSERT_MSG(archive_result.Succeeded(), "Could not open the PTM SharedExtSaveData archive!");
 
-        FileSys::Path gamecoin_path("gamecoin.dat");
+        FileSys::Path gamecoin_path("/gamecoin.dat");
         FileSys::Mode open_mode = {};
         open_mode.write_flag.Assign(1);
         open_mode.create_flag.Assign(1);