diff --git a/ISSUES/phase6-cleanup.md b/ISSUES/phase6-cleanup.md new file mode 100644 index 0000000..b330732 --- /dev/null +++ b/ISSUES/phase6-cleanup.md @@ -0,0 +1,115 @@ +# Phase 6 cleanup — unfinished items + +Leftover from the architectural refactor. All items are small, contained, and build on +the already-present `Result` + RAII infrastructure. + +## Status + +| # | Item | File(s) | Status | +|---|------|---------|--------| +| 1 | `AccountProfileHandle` RAII + apply in `account.cpp` | `handles.hpp`, `account.cpp` | [x] | +| 2 | `FILE*` in `account.cpp::iconPath` → `FileHandle` | `account.cpp` | [x] | +| 3 | Split `io::restore` into three focused functions | `io.cpp`, `io.hpp` | [x] | +| 4 | `sendAll`/`recvAll` → `nxst::Result` | `transfer_service.cpp` | dropped — callers don't propagate error, `bool` is correct | + +--- + +## Item 1 — `AccountProfileHandle` RAII + +**File:** `include/nxst/infra/fs/handles.hpp` + +Add after `FileHandle`: + +```cpp +// RAII wrapper for AccountProfile — auto-closes on destruction. +struct AccountProfileHandle { + AccountProfile profile{}; + bool valid{false}; + + AccountProfileHandle() = default; + ~AccountProfileHandle() { + if (valid) + accountProfileClose(&profile); + } + AccountProfileHandle(const AccountProfileHandle&) = delete; + AccountProfileHandle& operator=(const AccountProfileHandle&) = delete; + + AccountProfile* get() { return &profile; } +}; +``` + +**Apply in `account.cpp`:** + +`getUser()` (line ~62): replace raw `AccountProfile profile` + manual `accountProfileClose` with `AccountProfileHandle`. + +`iconPath()` (line ~98): same — wrap `AccountProfile profile` → `AccountProfileHandle`. + +--- + +## Item 2 — `FILE*` in `iconPath` → `FileHandle` + +**File:** `src/domain/account.cpp`, lines ~115-119 + +```cpp +// before +FILE* f = fopen(path, "wb"); +if (!f) return ""; +fwrite(buf.data(), 1, outSize, f); +fclose(f); + +// after +nxst::FileHandle f(fopen(path, "wb")); +if (!f) return ""; +fwrite(buf.data(), 1, outSize, f.get()); +``` + +No `fclose` needed — `FileHandle` dtor handles it. + +--- + +## Item 3 — Split `io::restore` + +**Current:** `io.cpp:236-317` — one 80-line function doing mount, validate, clear, copy, commit. + +**Split into three `static` helpers in `io.cpp` (not exposed in header):** + +``` +static nxst::Result clearSaveRoot(const std::string& dst_path) + — removes all files/dirs under save:/ + +static nxst::Result extractAndCommit(const std::string& src_path, const std::string& dst_path) + — copyDirectory + fsdevCommitDevice + +io::restore (public) + — mount, validate src, call clearSaveRoot, call extractAndCommit, return success +``` + +`io.hpp` declaration unchanged — `io::restore` signature stays the same. + +--- + +## Item 4 — `sendAll`/`recvAll` → `nxst::Result` + +**File:** `src/service/transfer_service.cpp`, lines 32-55 + +```cpp +// before +static bool sendAll(int sock, const void* buf, size_t len) { ... return true/false; } +static bool recvAll(int sock, void* buf, size_t len) { ... return true/false; } + +// after +static nxst::Result sendAll(int sock, const void* buf, size_t len); +static nxst::Result recvAll(int sock, void* buf, size_t len); +``` + +All callers: `if (!sendAll(...))` → `if (!sendAll(...).isOk())`. + +--- + +## Verification + +``` +cmake --build build/ # must produce NXST.elf + NXST.nro with zero warnings +``` + +No observable behavior change — pure refactor. diff --git a/include/nxst/infra/fs/handles.hpp b/include/nxst/infra/fs/handles.hpp index b2cefc8..ef8d1b8 100644 --- a/include/nxst/infra/fs/handles.hpp +++ b/include/nxst/infra/fs/handles.hpp @@ -49,4 +49,23 @@ struct FileHandle { } }; +// RAII wrapper for AccountProfile — auto-closes on destruction. +struct AccountProfileHandle { + AccountProfile profile{}; + bool valid{false}; + + AccountProfileHandle() = default; + ~AccountProfileHandle() { + if (valid) + accountProfileClose(&profile); + } // NOLINT(modernize-use-equals-default) + + AccountProfileHandle(const AccountProfileHandle&) = delete; + AccountProfileHandle& operator=(const AccountProfileHandle&) = delete; + + AccountProfile* get() { + return &profile; + } +}; + } // namespace nxst diff --git a/src/domain/account.cpp b/src/domain/account.cpp index 3ff62c7..40627f1 100644 --- a/src/domain/account.cpp +++ b/src/domain/account.cpp @@ -28,6 +28,7 @@ #include #include +#include static std::map mUsers; @@ -59,15 +60,15 @@ std::vector Account::ids(void) { static User getUser(AccountUid id) { User user{id, ""}; - AccountProfile profile; + nxst::AccountProfileHandle profile; AccountProfileBase profilebase; memset(&profilebase, 0, sizeof(profilebase)); - if (R_SUCCEEDED(accountGetProfile(&profile, id))) { - if (R_SUCCEEDED(accountProfileGet(&profile, NULL, &profilebase))) { + if (R_SUCCEEDED(accountGetProfile(profile.get(), id))) { + profile.valid = true; + if (R_SUCCEEDED(accountProfileGet(profile.get(), NULL, &profilebase))) { user.name = std::string(profilebase.nickname); } - accountProfileClose(&profile); } return user; } @@ -95,28 +96,24 @@ std::string Account::iconPath(AccountUid id) { mkdir("sdmc:/switch/NXST", 0755); mkdir("sdmc:/switch/NXST/cache", 0755); - AccountProfile profile; - if (R_FAILED(accountGetProfile(&profile, id))) + nxst::AccountProfileHandle profile; + if (R_FAILED(accountGetProfile(profile.get(), id))) return ""; + profile.valid = true; u32 imgSize = 0; - if (R_FAILED(accountProfileGetImageSize(&profile, &imgSize)) || imgSize == 0) { - accountProfileClose(&profile); + if (R_FAILED(accountProfileGetImageSize(profile.get(), &imgSize)) || imgSize == 0) return ""; - } std::vector buf(imgSize); u32 outSize = 0; - Result r = accountProfileLoadImage(&profile, buf.data(), imgSize, &outSize); - accountProfileClose(&profile); - if (R_FAILED(r) || outSize == 0) + if (R_FAILED(accountProfileLoadImage(profile.get(), buf.data(), imgSize, &outSize)) || outSize == 0) return ""; - FILE* f = fopen(path, "wb"); + nxst::FileHandle f(fopen(path, "wb")); if (!f) return ""; - fwrite(buf.data(), 1, outSize, f); - fclose(f); + fwrite(buf.data(), 1, outSize, f.get()); return std::string(path); } diff --git a/src/infra/fs/io.cpp b/src/infra/fs/io.cpp index 4769beb..9c86721 100644 --- a/src/infra/fs/io.cpp +++ b/src/infra/fs/io.cpp @@ -233,6 +233,40 @@ static void createSaveIfNeeded(u64 title_id, AccountUid uid) { fsCreateSaveDataFileSystem(&attr, &create_info, &meta); } +static nxst::Result clearSaveRoot(const std::string& dst_path) { + Directory save_root(dst_path); + for (size_t i = 0, sz = save_root.size(); i < sz; i++) { + if (save_root.folder(i)) { + io::deleteFolderRecursively(dst_path + save_root.entry(i) + "/"); + rmdir((dst_path + save_root.entry(i)).c_str()); + } else { + std::remove((dst_path + save_root.entry(i)).c_str()); + } + } + + Result res = fsdevCommitDevice("save"); + if (R_FAILED(res)) { + nxst::log::error("Failed to commit save after clearing with result 0x%08lX.", res); + return nxst::Result::failure("Failed to commit save after delete."); + } + return nxst::Result::success(); +} + +static nxst::Result extractAndCommit(const std::string& src_path, const std::string& dst_path) { + Result res = io::copyDirectory(src_path, dst_path); + if (R_FAILED(res)) { + nxst::log::error("Failed to copy %s to save:/ with result 0x%08lX.", src_path.c_str(), res); + return nxst::Result::failure("Failed to restore save."); + } + + res = fsdevCommitDevice("save"); + if (R_FAILED(res)) { + nxst::log::error("Failed to commit save with result 0x%08lX.", res); + return nxst::Result::failure("Failed to commit to save device."); + } + return nxst::Result::success(); +} + nxst::Result io::restore(size_t index, AccountUid uid, size_t cellIndex, const std::string& nameFromCell) { (void)cellIndex; @@ -276,37 +310,16 @@ nxst::Result io::restore(size_t index, AccountUid uid, size_t cellI } } - { - Directory save_root(dst_path); - for (size_t i = 0, sz = save_root.size(); i < sz; i++) { - if (save_root.folder(i)) { - io::deleteFolderRecursively(dst_path + save_root.entry(i) + "/"); - rmdir((dst_path + save_root.entry(i)).c_str()); - } else { - std::remove((dst_path + save_root.entry(i)).c_str()); - } - } + auto clear_res = clearSaveRoot(dst_path); + if (!clear_res.isOk()) { + FileSystem::unmount(); + return nxst::Result::failure(clear_res.error()); } - res = fsdevCommitDevice("save"); - if (R_FAILED(res)) { + auto extract_res = extractAndCommit(src_path, dst_path); + if (!extract_res.isOk()) { FileSystem::unmount(); - nxst::log::error("Failed to commit save after clearing with result 0x%08lX.", res); - return nxst::Result::failure("Failed to commit save after delete."); - } - - res = io::copyDirectory(src_path, dst_path); - if (R_FAILED(res)) { - FileSystem::unmount(); - nxst::log::error("Failed to copy %s to save:/ with result 0x%08lX.", src_path.c_str(), res); - return nxst::Result::failure("Failed to restore save."); - } - - res = fsdevCommitDevice("save"); - if (R_FAILED(res)) { - FileSystem::unmount(); - nxst::log::error("Failed to commit save with result 0x%08lX.", res); - return nxst::Result::failure("Failed to commit to save device."); + return nxst::Result::failure(extract_res.error()); } blinkLed(4);