Repository navigation
fix(libSceUserService): validate user login status in sceUserServiceGetGamePresets - #11087
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The code and regression cases are consistent with IsUserLoggedIn and the existing USER_SERVICE_ERROR_NOT_LOGGED_IN behavior: they cover unknown IDs, disconnected users, no output mutation on failure, null output precedence, and the disconnected-to-connected transition.
The Tested section only says the test was exercised via GuestUserService.cpp; it does not identify the OS/toolchain or a command/result, and the hosted windows product job is skipped. CONTRIBUTING.md requires tests to be run before opening a PR and the template asks what and where was checked. Please provide the focused test command and environment/results, or state explicitly that the test was not run. I cannot run it locally because full configuration stops at the unavailable SDL2 development package.
|
Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.
|
|
I manually checked the overlap bot’s shared-file warnings against the current heads. |
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed exact head 63b354ce2a1a86f3cbcbbe2afd2b617a7e79663c. The new login check matches the ordering used by sceUserServiceGetAgeLevel and the accessibility getters: a non-logged-in/unknown id returns USER_SERVICE_ERROR_NOT_LOGGED_IN before pointer validation, and leaves output unchanged. The added cases cover invalid ids, disconnected/connected secondary users, and null output precedence. I compiled the changed Export.cpp and GuestUserService.cpp with GCC 12.2 (-std=c++20 -Icore/libs -fsyntax-only); conventions and git diff --check upstream/main...HEAD pass. I could not run the test binary: full CMake configuration here stops because SDL2 development files are unavailable, and hosted product test jobs are skipped by path policy. The PR description identifies GuestUserService.cpp as its focused test.
|
Tested head
AI-assisted: yes (Claude Code); results above were run on this machine. |
|
@slopgobblercobbler I checked the new login and pointer ordering. Could you make the GamePresets debt entry explicit that this ordering and 0x80960009 are borrowed from GetAgeLevel and have not been measured for GetGamePresets? The current sentence reads as an established result for this call. Please retain the unmodelled preset values and the login-transition regression checks. |
|
Firmware data for the GamePresets debt entry. In 13.42's
I ran the export in an x86-64 emulator (unicorn) with that slot set to 0x4300 and the context NULL or a stand-in IPC object that logs the call and answers 0. Three runs gave identical output:
So on 13.42 a NULL Also, |
63b354c to
8431a9d
Compare
|
@BunnyFl Thank you so much for the 13.42 firmware disassembly analysis! I have updated the implementation so that the presets null check precedes the user login check (returning 0x80960005 for null pointers across both logged-in and unlogged users), updated the test suite in GuestUserService.cpp, and cited the firmware analysis in the debt entry. Verified clean with tools/check_conventions.py (0 errors). |
16d8bfa to
92c3635
Compare
|
Rebased cleanly onto latest |
|
Windows result for head Host: Windows 11 26300, Intel i7-12700K, WinLibs GCC 15.2.0 (x86_64-ucrt-posix-seh, r6), Release, The tree also carried the open PRs #11945, #11899, #11859, #11772, #10789, #11627, #11354, all merged cleanly, so the run is a joint one; the tests of this PR are AI-assisted: yes (Claude Code), the runs and the numbers above are from this machine. |
92c3635 to
d833409
Compare
MarcZX10
left a comment
There was a problem hiding this comment.
Re-checked — changes requested
The null-pointer precedence, login-state behavior, test preservation, and disassembly caveat are clearly documented. This branch is still 36 commits behind current main 52c09d567d2fa453494dd720fcf53a98149468e4; the contribution checklist requires a current-main base. The reported Windows run is on an older integration head.
What I ran
- Reviewed exact head
d833409fc2against currentmain. - Ran a current-main merge preview and
git diff --check. - Inspected the changed implementation/tests and the author/Windows test reports.
Results
| Check | Result |
|---|---|
| Current-main merge preview | Clean; current main is not in the branch history |
git diff --check |
Passed |
| Focused user-service tests | Windows report covers guest_user_service, user-name, and lifecycle tests on an older base |
| Current-main run | Not reported |
Please refresh onto current main and rerun guest_user_service on that exact head. I did not run the C++ tests locally.
d833409 to
deb8621
Compare
…eck in sceUserServiceGetGamePresets
deb8621 to
9c55d91
Compare
|
@maskym Thank you for running the Windows 11 test suite on the i7-12700K! I've updated the Tested section in the description with your test results. |
What
sceUserServiceGetGamePresets, returningUSER_SERVICE_ERROR_INVALID_ARGUMENT(0x80960005) whenpresets == nullptrper PS5 firmware 13.42 disassembly.presetsis valid, returningUSER_SERVICE_ERROR_NOT_LOGGED_IN(0x80960009) when querying a non-logged-in user.core/libs/prx/libSceUserService/Export.cppperCONVENTIONS.md:12.docs/dev/debt/libSceUserService.md.core/libs/tests/GuestUserService.cppcovering logged-in user, absent users (valid buffer and nullptr), and connected/disconnected user transitions.Tested
core/libs/tests/GuestUserService.cppcovering login transitions and parameter validation.guest_user_service,guest_user_service_user_name, andguest_user_service_lifecycleall passed (14/14 joint tests passed).python tools/check_conventions.py --base upstream/main --head HEAD: 0 findings.Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file.mdfiles or images added to the repository (attach them to this PR)