Skip to content

fix(libkernel): report POSIX rename errors from sceKernelRename - #2260

Closed
AchrafReyani wants to merge 2 commits into
boykopovar:mainfrom
AchrafReyani:fix/libkernel-rename-errors
Closed

AchrafReyani wants to merge 2 commits into
boykopovar:mainfrom
AchrafReyani:fix/libkernel-rename-errors

Conversation

@AchrafReyani

Copy link
Copy Markdown
Contributor

What

sceKernelRename returned ENOENT for a missing source and EIO for every other failure. On Windows the rename itself also went wrong: MinGW's std::filesystem::rename reports some missing paths as EIO and refuses to replace an existing empty directory, which FreeBSD's rename(2) does. Measured on main (d70b8998) with libstdc++'s std::filesystem::rename (generic category):

Case Windows std::filesystem::rename sceKernelRename on main FreeBSD rename(2)
source missing, target missing EIO ENOENT ENOENT
target's directory missing EIO EIO ENOENT
target's directory is a file EIO ENOTDIR
directory onto a file EIO ENOTDIR
file onto a directory EIO EISDIR
directory onto a non-empty directory EACCES EIO ENOTEMPTY
directory into its own subtree EIO EIO EINVAL
directory onto an empty directory EACCES fails with EIO replaces it

RenameError_nid_no_patch checks these rename(2) conditions before renaming and returns the guest error, the way sceKernelRmdir already checks its own. On Windows it removes the empty target directory first, so the replace succeeds as on the console. A rename onto the same file, including a case-only rename on Windows, still goes straight to std::filesystem::rename. Any other host failure is still EIO.

libc's rename has the same Windows behaviour (it calls std::filesystem::rename too). I left it alone because #1192 is moving that function into a new file; I can send the same fix for it afterwards.

Tested

Windows 11, WinLibs GCC 15.2.0 r7, Release, -DBUILD_TESTING=ON:

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes

🤖 Generated with Claude Code

sceKernelRename turned every failure after the source check into EIO,
and on Windows std::filesystem::rename itself reports a missing target
directory or a directory moved into itself as EIO and refuses to
replace an empty directory. Check the rename(2) error conditions before
renaming: ENOENT, ENOTDIR, EISDIR, ENOTEMPTY and EINVAL, and on Windows
remove the empty target directory first so it is replaced as on the
console.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

I found one error-mapping case to resolve before approval. symlink_status(source, error) and status(parent, error) set error when the path cannot be inspected (for example, permission_denied), but the helper only checks exists(status). A failed status then looks like file_type::not_found and is returned as guest ENOENT, masking the actual access failure. Please inspect error before mapping a not-found status, preserve the applicable POSIX error (or the documented EIO fallback), and add a regression where practical. The current PR checks skip full Linux/Windows Build, so they do not exercise this path.

@AchrafReyani

Copy link
Copy Markdown
Contributor Author

Full Linux and Windows builds and tests for this head (ef5a4f3e), using the project's Build workflow. I can't start the upstream manual run, so I ran the same build.yml in my fork through a mirror PR. Like the upstream run, the jobs check out the PR head merged into current main (d70b8998):

Job Result
linux success; full build, ctest: 516/516 passed
windows success; full build, ctest: 507/507 passed
relinker (Linux, Windows, macOS) success

Run: https://github.com/AchrafReyani/AnyPS5/actions/runs/37937885788

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

Thanks for the exact-head fork Build report. The Linux 516/516 and Windows 507/507 suites plus the relinker matrix are useful evidence and close the general build/test gap. The specific status-inspection case I raised remains untested by the listed sceKernelRename cases: when symlink_status(source, error) or status(parent, error) reports an inspection error, the helper still needs to distinguish that from a genuine not-found status before returning guest ENOENT. I am keeping that focused finding open; the full suites do not currently exercise permission-denied metadata inspection.

namespace fs = std::filesystem;
std::error_code error;
const auto sourceStatus = fs::symlink_status(source, error);
if (!fs::exists(sourceStatus)) return GUEST_ENOENT;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could check error before treating !exists(sourceStatus) as ENOENT? symlink_status can set permission_denied while the returned status is not_found, so this branch conflates an inspection failure with a genuinely missing source. The destination-parent and destination status checks below have the same pattern. Please preserve the applicable guest error (or the documented EIO fallback) when inspection fails.

symlink_status and status report a path that cannot be inspected with
an error and an unknown type, which the rename checks read as a
missing file. Return ENOTDIR and ENOENT only for a path that is not
found, EACCES when the inspection is denied and EIO otherwise.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@AchrafReyani

Copy link
Copy Markdown
Contributor Author

Good catch, thanks. Fixed in 2545929d:

  • sceKernelRename now reads the error that symlink_status/status set. libstdc++ also sets it for a plain miss (ENOENT/ENOTDIR with type not_found), so the mapping is:
    • not_a_directory → ENOTDIR;
    • any other not_found → ENOENT;
    • permission_denied → EACCES;
    • anything else → the EIO fallback.
  • The destination's status is checked the same way: absent is still fine, but any other inspection failure is returned instead of being read as "absent".
  • Regression (Linux, skipped as root): a directory with perms::none. Renaming its entry and renaming into it must both return 0x8002000D and leave the files in place. Windows has no comparable way to deny traversal, so the block is #ifndef _WIN32.

To show the test fails without the fix, I'm running it two ways on the fork's Build workflow: this head, and a control branch with only the new test on the previous head (ef5a4f3e). I'll post both results here.

@AchrafReyani

Copy link
Copy Markdown
Contributor Author

Results for 2545929d and the control. Both ran the project's build.yml (full builds plus ctest) in my fork, merged into main d70b8998:

Run Linux Windows
This head 2545929d 516/516 passed 507/507 passed
Control: the new test on the previous head ef5a4f3e failed, 1/516: guest_filesystem — Filesystem check failed at line 148 (guest errno 13) 507/507 passed (the check is #ifndef _WIN32)

Line 148 is the new perms::none check. Without the inspection fix, a source the kernel couldn't stat came back as ENOENT instead of EACCES. With it, both renames return 0x8002000D and the files stay where they were.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title is shown stopping on sceKernelRename's error codes or on the Windows empty-directory replace on main. Reopen with the title ID and the log line if one does.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants