{"action":"created","comment":{"url":"https://api.github.com/repos/citra-emu/citra/pulls/comments/85646346","pull_request_review_id":6352477,"id":85646346,"diff_hunk":"@@ -8,13 +8,307 @@\n #include \"common/logging/log.h\"\n #include \"core/file_sys/archive_sdmc.h\"\n #include \"core/file_sys/disk_archive.h\"\n+#include \"core/file_sys/path_parser.h\"\n #include \"core/settings.h\"\n \n ////////////////////////////////////////////////////////////////////////////////////////////////////\n // FileSys namespace\n \n namespace FileSys {\n \n+static const ResultCode ERROR_INVALID_PATH(ErrorDescription::FS_InvalidPath, ErrorModule::FS,\n+                                           ErrorSummary::InvalidArgument, ErrorLevel::Usage);\n+static const ResultCode ERROR_INVALID_OPEN_FLAGS(ErrorDescription::FS_InvalidOpenFlags,\n+                                                 ErrorModule::FS, ErrorSummary::Canceled,\n+                                                 ErrorLevel::Status);\n+static const ResultCode ERROR_NOT_FOUND(ErrorDescription::FS_NotFound, ErrorModule::FS,\n+                                        ErrorSummary::NotFound, ErrorLevel::Status);\n+static const ResultCode ERROR_UNEXPECTED_FILE_OR_DIRECTORY_SDMC(ErrorDescription::FS_NotAFile,\n+                                                                ErrorModule::FS,\n+                                                                ErrorSummary::Canceled,\n+                                                                ErrorLevel::Status);\n+static const ResultCode ERROR_ALREADY_EXISTS(ErrorDescription::FS_AlreadyExists, ErrorModule::FS,\n+                                             ErrorSummary::NothingHappened, ErrorLevel::Status);\n+\n+ResultVal<std::unique_ptr<FileBackend>> SDMCArchive::OpenFile(const Path& path,\n+                                                              const Mode mode) const {\n+    Mode modified_mode;\n+    modified_mode.hex = mode.hex;\n+\n+    // SDMC archive always opens a file with at least read permission\n+    modified_mode.read_flag.Assign(1);\n+\n+    return OpenFileBase(path, modified_mode);\n+}\n+\n+ResultVal<std::unique_ptr<FileBackend>> SDMCArchive::OpenFileBase(const Path& path,\n+                                                                  const Mode mode) const {\n+    LOG_DEBUG(Service_FS, \"called path=%s mode=%01X\", path.DebugStr().c_str(), mode.hex);\n+\n+    PathParser path_parser(path);\n+\n+    if (!path_parser.IsValid()) {\n+        LOG_ERROR(Service_FS, \"Invalid path %s\", path.DebugStr().c_str());\n+        return ERROR_INVALID_PATH;\n+    }\n+\n+    if (mode.hex == 0) {\n+        LOG_ERROR(Service_FS, \"Empty open mode\");\n+        return ERROR_INVALID_OPEN_FLAGS;\n+    }\n+\n+    if (mode.create_flag && !mode.write_flag) {\n+        LOG_ERROR(Service_FS, \"Create flag set but write flag not set\");\n+        return ERROR_INVALID_OPEN_FLAGS;\n+    }\n+\n+    auto full_path = path_parser.BuildHostPath(mount_point);\n+\n+    switch (path_parser.GetHostStatus(mount_point)) {\n+    case PathParser::InvalidMountPoint:\n+        LOG_CRITICAL(Service_FS, \"Invalid mount point %s\", mount_point.c_str());\n+        return ERROR_NOT_FOUND;\n+    case PathParser::PathNotFound:\n+    case PathParser::FileInPath:\n+        LOG_ERROR(Service_FS, \"Path not found %s\", full_path.c_str());\n+        return ERROR_NOT_FOUND;\n+    case PathParser::DirectoryFound:\n+        LOG_ERROR(Service_FS, \"%s is not a file\", full_path.c_str());\n+        return ERROR_UNEXPECTED_FILE_OR_DIRECTORY_SDMC;\n+    case PathParser::NotFound:\n+        if (!mode.create_flag) {\n+            LOG_ERROR(Service_FS, \"Non-existing file %s can't be open without mode create.\",\n+                      full_path.c_str());\n+            return ERROR_NOT_FOUND;\n+        } else {\n+            // Create the file\n+            FileUtil::CreateEmptyFile(full_path);\n+        }\n+        break;\n+    }\n+\n+    std::string mode_string = \"\";\n+    if (mode.write_flag)\n+        mode_string += \"r+\";","path":"src/core/file_sys/archive_sdmc.cpp","position":85,"original_position":85,"commit_id":"998c15f070529efe4d9c5f360158a748522bb072","original_commit_id":"998c15f070529efe4d9c5f360158a748522bb072","user":{"login":"bunnei","id":6956688,"avatar_url":"https://avatars.githubusercontent.com/u/6956688?v=3","gravatar_id":"","url":"https://api.github.com/users/bunnei","html_url":"https://github.com/bunnei","followers_url":"https://api.github.com/users/bunnei/followers","following_url":"https://api.github.com/users/bunnei/following{/other_user}","gists_url":"https://api.github.com/users/bunnei/gists{/gist_id}","starred_url":"https://api.github.com/users/bunnei/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bunnei/subscriptions","organizations_url":"https://api.github.com/users/bunnei/orgs","repos_url":"https://api.github.com/users/bunnei/repos","events_url":"https://api.github.com/users/bunnei/events{/privacy}","received_events_url":"https://api.github.com/users/bunnei/received_events","type":"User","site_admin":false},"body":"`mode_string = \"r+\";`","created_at":"2016-10-29T19:14:14Z","updated_at":"2016-10-29T19:14:14Z","html_url":"https://github.com/citra-emu/citra/pull/2132#discussion_r85646346","pull_request_url":"https://api.github.com/repos/citra-emu/citra/pulls/2132","_links":{"self":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/comments/85646346"},"html":{"href":"https://github.com/citra-emu/citra/pull/2132#discussion_r85646346"},"pull_request":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/2132"}}},"pull_request":{"url":"https://api.github.com/repos/citra-emu/citra/pulls/2132","id":90475478,"html_url":"https://github.com/citra-emu/citra/pull/2132","diff_url":"https://github.com/citra-emu/citra/pull/2132.diff","patch_url":"https://github.com/citra-emu/citra/pull/2132.patch","issue_url":"https://api.github.com/repos/citra-emu/citra/issues/2132","number":2132,"state":"open","locked":false,"title":"Correct FS error codes & add path boundary checks","user":{"login":"wwylele","id":4592895,"avatar_url":"https://avatars.githubusercontent.com/u/4592895?v=3","gravatar_id":"","url":"https://api.github.com/users/wwylele","html_url":"https://github.com/wwylele","followers_url":"https://api.github.com/users/wwylele/followers","following_url":"https://api.github.com/users/wwylele/following{/other_user}","gists_url":"https://api.github.com/users/wwylele/gists{/gist_id}","starred_url":"https://api.github.com/users/wwylele/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wwylele/subscriptions","organizations_url":"https://api.github.com/users/wwylele/orgs","repos_url":"https://api.github.com/users/wwylele/repos","events_url":"https://api.github.com/users/wwylele/events{/privacy}","received_events_url":"https://api.github.com/users/wwylele/received_events","type":"User","site_admin":false},"body":"The main part of this PR is to fix error codes of archive operations. Because error codes can vary among different error types and archive types, I had to refactor the `ArchiveBackend` interface and their subclass: let all method report its own error code, and split `DiskArchive` into 3 classes `SaveDataArchive`, `ExtSaveDataArchive` and `SDMCArchive`. Sorry for not following the suggestion in the code comment \"This should be subclassed by concrete archive types\", but if, for example, I let `SaveDataArchive` be a subclass off `DiskArchive`, I will literally override everything due to different error check behaviour, which is effectively the same as making a brand new class.\r\n\r\nThe error codes were got from [this test program](https://github.com/wwylele/3DS-FS-Tester) ([compiled version](https://github.com/wwylele/3DS-FS-Tester/releases/download/1.0/wwylele_fs_tester.cxi)) and here is [a summary sheet](https://drive.google.com/file/d/0ByjS9HUStkSXUmtPdWdmd25CeGc/view?usp=drivesdk). However, with this PR, citra and real 3DS still don't behave exactly the same for some very edge cases that are unlikely to happen, such as `OpenFile(\"/\")`. These cases are marked red in the sheet. Implementing those edge cases will make the code too complicated but doesn't contribute much to the compatibility, and I feel like they can be even indeterminate and are \"undefined behaviour\" on real 3DS. If anyone wants citra to be more accurate, they can always add more error checks in the future.\r\n\r\nNote that I didn't test `RenameFile` and `RenameDirectory`, and I keep the old error code. I want to leave this to the next PR, because they are another large topic.\r\n\r\nYou can also run the mentioned test program on citra with this PR to test. However, the program uses software keyboard to input commands, so you may want to merge [this commit](https://github.com/wwylele/citra/commit/0baf9f8fb6f0cd160d735c59d44a43d005539e2d) so that you can input commands from stdin.\r\n\r\nFixes #2094, and \"path boundary check\" comments in #2106.\r\n\r\nAdditional commits:\r\n - hwtested and implemented SDMC write-only archive. Should fixes #1607.\r\n - rename `SaveDataCheck` archive to `NCCH` archive, according to [3dbrew](https://www.3dbrew.org/wiki/Filesystem_services#Archives) and its behaviour.\r\n\r\nThis PR is not widely test yet. Please help me test games to see if there is any regression! Please especially pay attention when game want to create a new save or delete the save.","created_at":"2016-10-22T08:57:57Z","updated_at":"2016-10-29T19:14:14Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f9e1923ca2b4856ac7c8d1b8f11c635727ef280f","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/citra-emu/citra/pulls/2132/commits","review_comments_url":"https://api.github.com/repos/citra-emu/citra/pulls/2132/comments","review_comment_url":"https://api.github.com/repos/citra-emu/citra/pulls/comments{/number}","comments_url":"https://api.github.com/repos/citra-emu/citra/issues/2132/comments","statuses_url":"https://api.github.com/repos/citra-emu/citra/statuses/998c15f070529efe4d9c5f360158a748522bb072","head":{"label":"wwylele:fix-fs-err","ref":"fix-fs-err","sha":"998c15f070529efe4d9c5f360158a748522bb072","user":{"login":"wwylele","id":4592895,"avatar_url":"https://avatars.githubusercontent.com/u/4592895?v=3","gravatar_id":"","url":"https://api.github.com/users/wwylele","html_url":"https://github.com/wwylele","followers_url":"https://api.github.com/users/wwylele/followers","following_url":"https://api.github.com/users/wwylele/following{/other_user}","gists_url":"https://api.github.com/users/wwylele/gists{/gist_id}","starred_url":"https://api.github.com/users/wwylele/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wwylele/subscriptions","organizations_url":"https://api.github.com/users/wwylele/orgs","repos_url":"https://api.github.com/users/wwylele/repos","events_url":"https://api.github.com/users/wwylele/events{/privacy}","received_events_url":"https://api.github.com/users/wwylele/received_events","type":"User","site_admin":false},"repo":{"id":53776373,"name":"citra","full_name":"wwylele/citra","owner":{"login":"wwylele","id":4592895,"avatar_url":"https://avatars.githubusercontent.com/u/4592895?v=3","gravatar_id":"","url":"https://api.github.com/users/wwylele","html_url":"https://github.com/wwylele","followers_url":"https://api.github.com/users/wwylele/followers","following_url":"https://api.github.com/users/wwylele/following{/other_user}","gists_url":"https://api.github.com/users/wwylele/gists{/gist_id}","starred_url":"https://api.github.com/users/wwylele/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/wwylele/subscriptions","organizations_url":"https://api.github.com/users/wwylele/orgs","repos_url":"https://api.github.com/users/wwylele/repos","events_url":"https://api.github.com/users/wwylele/events{/privacy}","received_events_url":"https://api.github.com/users/wwylele/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/wwylele/citra","description":"Nintendo 3DS Emulator","fork":true,"url":"https://api.github.com/repos/wwylele/citra","forks_url":"https://api.github.com/repos/wwylele/citra/forks","keys_url":"https://api.github.com/repos/wwylele/citra/keys{/key_id}","collaborators_url":"https://api.github.com/repos/wwylele/citra/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/wwylele/citra/teams","hooks_url":"https://api.github.com/repos/wwylele/citra/hooks","issue_events_url":"https://api.github.com/repos/wwylele/citra/issues/events{/number}","events_url":"https://api.github.com/repos/wwylele/citra/events","assignees_url":"https://api.github.com/repos/wwylele/citra/assignees{/user}","branches_url":"https://api.github.com/repos/wwylele/citra/branches{/branch}","tags_url":"https://api.github.com/repos/wwylele/citra/tags","blobs_url":"https://api.github.com/repos/wwylele/citra/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/wwylele/citra/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/wwylele/citra/git/refs{/sha}","trees_url":"https://api.github.com/repos/wwylele/citra/git/trees{/sha}","statuses_url":"https://api.github.com/repos/wwylele/citra/statuses/{sha}","languages_url":"https://api.github.com/repos/wwylele/citra/languages","stargazers_url":"https://api.github.com/repos/wwylele/citra/stargazers","contributors_url":"https://api.github.com/repos/wwylele/citra/contributors","subscribers_url":"https://api.github.com/repos/wwylele/citra/subscribers","subscription_url":"https://api.github.com/repos/wwylele/citra/subscription","commits_url":"https://api.github.com/repos/wwylele/citra/commits{/sha}","git_commits_url":"https://api.github.com/repos/wwylele/citra/git/commits{/sha}","comments_url":"https://api.github.com/repos/wwylele/citra/comments{/number}","issue_comment_url":"https://api.github.com/repos/wwylele/citra/issues/comments{/number}","contents_url":"https://api.github.com/repos/wwylele/citra/contents/{+path}","compare_url":"https://api.github.com/repos/wwylele/citra/compare/{base}...{head}","merges_url":"https://api.github.com/repos/wwylele/citra/merges","archive_url":"https://api.github.com/repos/wwylele/citra/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/wwylele/citra/downloads","issues_url":"https://api.github.com/repos/wwylele/citra/issues{/number}","pulls_url":"https://api.github.com/repos/wwylele/citra/pulls{/number}","milestones_url":"https://api.github.com/repos/wwylele/citra/milestones{/number}","notifications_url":"https://api.github.com/repos/wwylele/citra/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/wwylele/citra/labels{/name}","releases_url":"https://api.github.com/repos/wwylele/citra/releases{/id}","deployments_url":"https://api.github.com/repos/wwylele/citra/deployments","created_at":"2016-03-13T09:09:07Z","updated_at":"2016-10-22T23:02:50Z","pushed_at":"2016-10-28T05:21:13Z","git_url":"git://github.com/wwylele/citra.git","ssh_url":"git@github.com:wwylele/citra.git","clone_url":"https://github.com/wwylele/citra.git","svn_url":"https://github.com/wwylele/citra","homepage":"https://citra-emu.org","size":14026,"stargazers_count":9,"watchers_count":9,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":9,"default_branch":"master"}},"base":{"label":"citra-emu:master","ref":"master","sha":"eac57e29c1c6eaa0411874ae4b03fa8739772774","user":{"login":"citra-emu","id":7605670,"avatar_url":"https://avatars.githubusercontent.com/u/7605670?v=3","gravatar_id":"","url":"https://api.github.com/users/citra-emu","html_url":"https://github.com/citra-emu","followers_url":"https://api.github.com/users/citra-emu/followers","following_url":"https://api.github.com/users/citra-emu/following{/other_user}","gists_url":"https://api.github.com/users/citra-emu/gists{/gist_id}","starred_url":"https://api.github.com/users/citra-emu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citra-emu/subscriptions","organizations_url":"https://api.github.com/users/citra-emu/orgs","repos_url":"https://api.github.com/users/citra-emu/repos","events_url":"https://api.github.com/users/citra-emu/events{/privacy}","received_events_url":"https://api.github.com/users/citra-emu/received_events","type":"Organization","site_admin":false},"repo":{"id":19866730,"name":"citra","full_name":"citra-emu/citra","owner":{"login":"citra-emu","id":7605670,"avatar_url":"https://avatars.githubusercontent.com/u/7605670?v=3","gravatar_id":"","url":"https://api.github.com/users/citra-emu","html_url":"https://github.com/citra-emu","followers_url":"https://api.github.com/users/citra-emu/followers","following_url":"https://api.github.com/users/citra-emu/following{/other_user}","gists_url":"https://api.github.com/users/citra-emu/gists{/gist_id}","starred_url":"https://api.github.com/users/citra-emu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/citra-emu/subscriptions","organizations_url":"https://api.github.com/users/citra-emu/orgs","repos_url":"https://api.github.com/users/citra-emu/repos","events_url":"https://api.github.com/users/citra-emu/events{/privacy}","received_events_url":"https://api.github.com/users/citra-emu/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/citra-emu/citra","description":"Nintendo 3DS Emulator","fork":false,"url":"https://api.github.com/repos/citra-emu/citra","forks_url":"https://api.github.com/repos/citra-emu/citra/forks","keys_url":"https://api.github.com/repos/citra-emu/citra/keys{/key_id}","collaborators_url":"https://api.github.com/repos/citra-emu/citra/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/citra-emu/citra/teams","hooks_url":"https://api.github.com/repos/citra-emu/citra/hooks","issue_events_url":"https://api.github.com/repos/citra-emu/citra/issues/events{/number}","events_url":"https://api.github.com/repos/citra-emu/citra/events","assignees_url":"https://api.github.com/repos/citra-emu/citra/assignees{/user}","branches_url":"https://api.github.com/repos/citra-emu/citra/branches{/branch}","tags_url":"https://api.github.com/repos/citra-emu/citra/tags","blobs_url":"https://api.github.com/repos/citra-emu/citra/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/citra-emu/citra/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/citra-emu/citra/git/refs{/sha}","trees_url":"https://api.github.com/repos/citra-emu/citra/git/trees{/sha}","statuses_url":"https://api.github.com/repos/citra-emu/citra/statuses/{sha}","languages_url":"https://api.github.com/repos/citra-emu/citra/languages","stargazers_url":"https://api.github.com/repos/citra-emu/citra/stargazers","contributors_url":"https://api.github.com/repos/citra-emu/citra/contributors","subscribers_url":"https://api.github.com/repos/citra-emu/citra/subscribers","subscription_url":"https://api.github.com/repos/citra-emu/citra/subscription","commits_url":"https://api.github.com/repos/citra-emu/citra/commits{/sha}","git_commits_url":"https://api.github.com/repos/citra-emu/citra/git/commits{/sha}","comments_url":"https://api.github.com/repos/citra-emu/citra/comments{/number}","issue_comment_url":"https://api.github.com/repos/citra-emu/citra/issues/comments{/number}","contents_url":"https://api.github.com/repos/citra-emu/citra/contents/{+path}","compare_url":"https://api.github.com/repos/citra-emu/citra/compare/{base}...{head}","merges_url":"https://api.github.com/repos/citra-emu/citra/merges","archive_url":"https://api.github.com/repos/citra-emu/citra/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/citra-emu/citra/downloads","issues_url":"https://api.github.com/repos/citra-emu/citra/issues{/number}","pulls_url":"https://api.github.com/repos/citra-emu/citra/pulls{/number}","milestones_url":"https://api.github.com/repos/citra-emu/citra/milestones{/number}","notifications_url":"https://api.github.com/repos/citra-emu/citra/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/citra-emu/citra/labels{/name}","releases_url":"https://api.github.com/repos/citra-emu/citra/releases{/id}","deployments_url":"https://api.github.com/repos/citra-emu/citra/deployments","created_at":"2014-05-16T18:33:15Z","updated_at":"2016-10-29T18:56:51Z","pushed_at":"2016-10-29T16:56:22Z","git_url":"git://github.com/citra-emu/citra.git","ssh_url":"git@github.com:citra-emu/citra.git","clone_url":"https://github.com/citra-emu/citra.git","svn_url":"https://github.com/citra-emu/citra","homepage":"https://citra-emu.org","size":22955,"stargazers_count":2394,"watchers_count":2394,"language":"C++","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":621,"mirror_url":null,"open_issues_count":224,"forks":621,"open_issues":224,"watchers":2394,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/2132"},"html":{"href":"https://github.com/citra-emu/citra/pull/2132"},"issue":{"href":"https://api.github.com/repos/citra-emu/citra/issues/2132"},"comments":{"href":"https://api.github.com/repos/citra-emu/citra/issues/2132/comments"},"review_comments":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/2132/comments"},"review_comment":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/citra-emu/citra/pulls/2132/commits"},"statuses":{"href":"https://api.github.com/repos/citra-emu/citra/statuses/998c15f070529efe4d9c5f360158a748522bb072"}}}}
{"actor":{"display_login":"bunnei"}}