Repository navigation
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
Reviewed the scalar Cp1252/Cp1251/Cp1250 changes at head b41fdbc. The table entries marked unassigned now fail in both conversion directions, and the UTF-8 decoder’s lengths/errors for truncated sequences, malformed continuations, overlong encodings, surrogates, and out-of-table values match the measurements described. The 42 measured-sequence tests and byte-table round trips are appropriately focused. This exact commit is included in the descendant #10186 C++20 syntax/conventions check; hosted eligibility, conventions, intake, labels and report pass, while product builds are path-skipped. The author reports the ten CES tests passing three consecutive runs. I could not run the executable locally because full CMake configuration is blocked by missing SDL2 development files.
MarcZX10
left a comment
There was a problem hiding this comment.
Correction to my prior approval: I had not evaluated the direct overlap with the newly refreshed #10056 head. git merge-tree of #10183 and #10056 reports a content conflict in core/libs/prx/libSceCesCs/Export.cpp, where both alter the single-byte decode/error behavior. Please coordinate the landing order with #10056's author, declare the dependency on the checklist if this PR lands second, and preserve both changes after rebasing. #10185 and #10186 already stack on this conversion work, so keep that dependency chain intact. The exact-head tests previously reviewed remain useful; hosted product checks are path-skipped. This is a keep-both sequencing request, not a request to close either PR.
MarcZX10
left a comment
There was a problem hiding this comment.
Re-review of exact head 8e10ddbb5ce16446c68d71054b6a9ffec76049d6. The previous review asked for an agreed order with #10056 because both changed the single-byte conversion behavior. The authors have now explicitly agreed that #10183/#10184 land first, followed by #10185 and #10186, then #10056; #10056 records its dependency on #10186, and the CES stack PRs record their dependencies. The #10183 checklist correctly remains Depends on: none because it is first.
The implementation diff against current main is limited to Export.cpp, the CES tests, and the CES debt entry; the new commit only merges the same current main base. Exact-head conventions and git diff --check pass. I also ran GCC C++20 -Wall -Wextra -Werror -fsyntax-only on Export.cpp successfully. Hosted eligibility, conventions, labels and report pass; product test/build jobs are skipped by path policy. The author-reported CES test runs remain the available runtime evidence; I could not run the full CMake test locally. The prior overlap blocker is resolved. Approving this exact head.
|
@oneandonlydean this was closed a second after #9107 merged, without a note, so I wanted to check whether that was intended. #9107 adds the Korean and Big5 profiles; this PR changes the single-byte conversion the PS5 measurement showed, which #9107 does not cover. The head still merges cleanly into current |
|
@orezraey #9210 has landed and changes |
|
Merged |
…profiles Only the follow-up commit b4670f6 (8 getters, sparse tables, Unassigned rejection in the shared lookup); the carried measured-conversion stack (boykopovar#10183-boykopovar#10186 via ddc4ab9) and upstream merges are left out, so our sbcToUnicode callers return UNASSIGNED_CODE for holes and GuestCesSparseProfiles expects our conversion semantics (no terminators, lengths untouched on failure); debt doc: duplicate single-byte, pending-export and EUC-KR bullets merged, pending count 51.
|
@rooklz: #9175, #10430 and #10431 change the same lines as this PR in |
|
@orezraey data from a second console that may help this PR: I swept
Every single-byte profile also answers |
|
@rooklz thanks, a second console agreeing on the Cp1252, Cp1251 and Cp1250 holes is useful evidence for this PR. I would keep this PR to those three measured profiles, since #10184, #10185 and #10186 stack on it and it already waits on several overlapping PRs. The Cp1254 and Cp1258 holes and the |
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Everything in this PR landed with #10185, which contained this branch. I merged |
|
Thanks! After the latest merge from main this PR has no changes left: the conversion work landed with #10185. Closing it as done. |
What
sceCesSbcToUtf8andsceCesUtf8ToSbcused assumed tables and errors. I measured them on a retail PS5 (firmware 13.60) with homebrew probes, from a payload process loadinglibSceCesCs.sprx. The sources and console logs are in this gist (pces3topces6).Tables. All 128 high bytes of Cp1252, Cp1251 and Cp1250 match the console, except the bytes the console leaves unassigned (
0x805C0020), which we mapped to C1 control characters:They are unassigned in both directions now (
U+0081and the others no longer map back), and the MBCS string conversions that share these tables follow. The console'ssceCesMbcsStrToUtf8Stralso returned0x805C0020for Cp1252 byte 0x81.sceCesSbcToUtf8.0x805C0004(we returned0x805C0001).0x805C0020before the output is checked.0x805C0030) or too small (0x805C0031).sceCesUtf8ToSbc.0x805C0010(we returned0x805C0011for a zero size).0xF8–0xFBand0xFC–0xFDtake 5 and 6 bytes;0xFEand0xFFreturn0x805C0014with length 0;0x805C0011with the full length, unless a byte already present is not a continuation byte;0x805C0014with the length set to that byte's index, or the index plus one when the sequence is also short;0x805C0015(we returned0x805C0014).0x805C0004.0x805C0020.0x805C0030last.The test pins 42 measured sequences with their results and lengths.
Not changed. The console reads any non-null profile pointer as a table: a zero-filled buffer and an EUC-JP profile both converted. Here a pointer that is not one of the three profiles still returns
0x805C0001. Which comes first between a null profile and a scalar above U+10FFFF was not measured. The UTF-8 decoder of the string conversions is unchanged, since only the scalar functions were measured. The debt entries record all of this.Also found: on this firmware
libSceCesCs.sprxdoes not export the Unicode scalar functions inUnicode.cpp(sceCesUtf8ToUtf32,sceCesUtf16ToUtf8,sceCesUcs2ToUtf8and the others;pces2), so they could not be measured.On top of #8923 and #8972. The 27 new profiles keep their tables. The UTF-16 and UTF-32 forms from #8972 share the lookup, so they now answer
UNASSIGNED_CODEfor the unassigned bytes too, andguest_ces_wideexpects that for Cp12510x98.Tested
main(f26434afa3, with refactor(libs): add full export headers for libs #10706):f7f39ad89c. The test now takes its declarations fromlibSceCesCs/include/Api.hpp(sceCesRefersUcsProfileCp1250returnsconst void*there, so the test casts it). The 14guest_ces*tests pass three runs in a row on Windows (MinGW) and on Linux (WSL Ubuntu, GCC);public_headersaudit andcheck_conventionspass.Windows 11, MinGW (winlibs).
guest_ceschecks:0x805C0020with length 0 for the unassigned ones;sceCesUtf8ToSbcsequences;mainand passes with the change. The tenguest_ces*tests pass three runs in a row.tools/check_conventions.pypasses.tools/pr_overlap.pylists feat(libSceCesCs): add 27 single-byte code page profiles #8923, feat(libSceCesCs): add the Windows-1253, 1255 and 1257 profiles #9912 and feat(libSceCesCs): add the GB18030-2000 and GB18030-2005 profiles #9210, which add other code pages and reworksbcToUnicode. They do not change these two functions or these three tables, but whichever merges second needs a rebase. It also lists PRs that touch other lines ofdocs/dev/debt/libSceCesCs.md.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)