refactor: phase 6 cleanup — RAII handles and io::restore split
- AccountProfileHandle RAII wrapper in handles.hpp; applied in account.cpp (getUser, iconPath) replacing manual accountProfileClose - FILE* in iconPath replaced with existing FileHandle RAII wrapper - io::restore split into clearSaveRoot + extractAndCommit static helpers; public signature unchanged - sendAll/recvAll kept as bool — callers don't propagate the error string, Result<void> would add noise without benefit Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -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<T>` + 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<void>` | `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<void> clearSaveRoot(const std::string& dst_path)
|
||||
— removes all files/dirs under save:/
|
||||
|
||||
static nxst::Result<void> 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<void>`
|
||||
|
||||
**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<void> sendAll(int sock, const void* buf, size_t len);
|
||||
static nxst::Result<void> 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.
|
||||
@@ -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
|
||||
|
||||
+12
-15
@@ -28,6 +28,7 @@
|
||||
#include <sys/stat.h>
|
||||
|
||||
#include <nxst/domain/account.hpp>
|
||||
#include <nxst/infra/fs/handles.hpp>
|
||||
|
||||
static std::map<AccountUid, User> mUsers;
|
||||
|
||||
@@ -59,15 +60,15 @@ std::vector<AccountUid> 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<u8> 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);
|
||||
}
|
||||
|
||||
|
||||
+41
-28
@@ -233,6 +233,40 @@ static void createSaveIfNeeded(u64 title_id, AccountUid uid) {
|
||||
fsCreateSaveDataFileSystem(&attr, &create_info, &meta);
|
||||
}
|
||||
|
||||
static nxst::Result<void> 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<void>::failure("Failed to commit save after delete.");
|
||||
}
|
||||
return nxst::Result<void>::success();
|
||||
}
|
||||
|
||||
static nxst::Result<void> 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<void>::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<void>::failure("Failed to commit to save device.");
|
||||
}
|
||||
return nxst::Result<void>::success();
|
||||
}
|
||||
|
||||
nxst::Result<std::string> io::restore(size_t index, AccountUid uid, size_t cellIndex,
|
||||
const std::string& nameFromCell) {
|
||||
(void)cellIndex;
|
||||
@@ -276,37 +310,16 @@ nxst::Result<std::string> 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<std::string>::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<std::string>::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<std::string>::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<std::string>::failure("Failed to commit to save device.");
|
||||
return nxst::Result<std::string>::failure(extract_res.error());
|
||||
}
|
||||
|
||||
blinkLed(4);
|
||||
|
||||
Reference in New Issue
Block a user