Skip to content

fix(json): implement allocator error notification - #2308

Closed
yeahboi233 wants to merge 1 commit into
boykopovar:mainfrom
yeahboi233:fix/json-allocator-error-notification
Closed

yeahboi233 wants to merge 1 commit into
boykopovar:mainfrom
yeahboi233:fix/json-allocator-error-notification

Conversation

@yeahboi233

@yeahboi233 yeahboi233 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Calling the existing sce::Json::MemAllocator::notifyError export raises a not-implemented exception instead of reporting the allocation error. Implement its default notification by printing the error code and requested byte count to stdout, returning normally and leaving allocator/user-data storage untouched.

Replace the parameterless, integer-returning placeholder with a void callback taking the implicit allocator pointer, error code, 64-bit requested size and user-data pointer. The argument types come from the existing PS5 mangled export; the void return and default printing behavior come from the related VitaSDK JSON API. TechnicalDebt records that the PS5 return contract and exact diagnostic wording remain unverified on hardware.

This implements one existing gray cell in the JSON library: 92/94 to 93/94 functions in the progress report. It does not change JSON allocation or dispatch custom allocator callbacks. #992 originally declared this throwing import. No title-compatibility or unresolved-import fix is claimed.

Tested

macOS 26.6.2, Apple Silicon, AppleClang 21, current main d70b89989473ba1f6ae13e44e079e67f1f8a44b0:

  • The new checks in the existing GuestJson.cpp test fail on the original production stub with its not-implemented exception.
  • Production Export.cpp plus the complete GuestJson.cpp passes in an arm64 Release build (-O2 -DNDEBUG), arm64 AddressSanitizer/UBSan build, and x86-64 Release build executed through the already installed Rosetta.
  • Five notification cases cover signed error limits, zero size, a size beyond 32 bits, maximum size_t, null/non-null user data, diagnostic contents, normal return and unchanged opaque storage. Existing JSON checks still run.
  • Direct Mac builds link an external test-only implementation of NotImplemented_nid_no_patch that throws for the remaining unrelated RTTI stub. They exercise the production JSON source; they do not constitute the full runtime build or patched .prx loader validation.
  • Convention checks and git diff --check pass. The existing guest_json CTest registration runs the added checks in a full build.

Full Linux/Windows builds and console/game execution have not been run locally. Native Windows stdout capture and the guest calling convention need their platform checks.

Checklist

  • Based on current main; open-PR overlap checked before submission, with the later concurrent feat(libSceJson2): implement InitParameterRtti and MemAllocator::notifyError #2309 overlap recorded below
  • 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 (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • AI-assisted: yes; implementation, tests and description were written with Codex assistance

Open-PR overlap

The overlap bot reports #2281 and #2309. Neither is a dependency of this implementation.

Depends on: none remains accurate. The shared export is a competing implementation requiring a maintainer choice or removal from one PR; declaring a dependency solely to suppress the bot would not reconcile the behavior.

@github-actions

github-actions Bot commented Oct 9, 2026

Copy link
Copy Markdown

Progress report (d70b899 → a979c66)

📈 System libraries: 84.72% (+0.03%, +1 implemented functions)

✅ 1 implemented
Library Name
libSceJson2 _ZN3sce4Json12MemAllocator11notifyErrorEimPv

@yeahboi233
yeahboi233 marked this pull request as ready for review October 9, 2026 15:19
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#2281 docs/dev/TechnicalDebt.md L59
#2309 core/libs/tests/GuestJson.cpp L88

@yeahboi233

Copy link
Copy Markdown
Contributor Author

Following up on the overlap report:

  • feat(net): implement synchronous resolver abort state #2281 conflicts only in the one-line unknown-signature list in TechnicalDebt.md: it removes sceNetResolverAbort, while this PR removes notifyError. The implementation and tests are independent. After either merges, I can rebase this PR and preserve both removals and both functions' limitation notes.
  • feat(libSceJson2): implement InitParameterRtti and MemAllocator::notifyError #2309 is a concurrent implementation of the same notifyError export. Its current implementation discards the error code and requested size without printing a notification. This PR prints the default diagnostic described by the related VitaSDK JSON API and tests the diagnostic, argument limits and unchanged opaque storage. The PS5-specific uncertainty remains documented. This PR leaves the unverified RTTI constructor unimplemented.

Neither PR is a dependency, so Depends on: none remains accurate. A maintainer choice or removal of the duplicated export from one PR is needed before both can merge.

The local arm64 Release, ASan/UBSan and x86-64 Rosetta JSON tests passed, as did the relinker CI matrix and conventions checks. Full Linux/Windows library builds were skipped by the automatic workflow. Could a collaborator run those full checks and review which notification implementation to retain?

@AnzouBelilita

Copy link
Copy Markdown

Independent Windows validation for PR #2308:

  • OS: Windows 10 Pro, build 19045
  • Toolchain: MinGW-w64 GCC 16.2.0, CMake 4.4.4, Ninja 1.13.2
  • Configured the project successfully in Release mode with BUILD_TESTING=ON.
  • Built the guest_json_tests target successfully (96 build steps).
  • CTest: guest_json passed (1/1, 0 failures).
  • python tools/check_conventions.py --base d70b89989473ba1f6ae13e44e079e67f1f8a44b0 passed.
  • git diff --check d70b89989473ba1f6ae13e44e079e67f1f8a44b0...HEAD passed.

This independently confirms that the PR builds and its JSON tests pass on native Windows. It does not verify the PS5-specific calling convention or exact behavior on console hardware.

No production code was modified during validation.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing: no title calls MemAllocator::notifyError on main, so the only effect is +1 in the progress report. Please reopen with the title ID and the log line if one reaches it.

ycbuzi pushed a commit to ycbuzi/AnyPS5 that referenced this pull request Oct 10, 2026
…ication

Harvested from upstream (closed unmerged); library completion 2885/3260 -> 2886/3260. (changelog conflict: kept both)

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011krDVCnnMQPaEFEqCDnKmA
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