Repository navigation
Conversation
|
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.
|
MarcZX10
left a comment
There was a problem hiding this comment.
I reviewed head 6cdc37108d03603d0dbd950e9e2f8a5109e7ea84 against current main and found regressions/overlap:
sceSslGetPemcopiescrt->rawbytes directly and reports the DER length. The existingGuestSslLoad.cpptest requires PEM output (including the PEM size query, PEM text and short-buffer error), and the currentmainimplementation atsceSslGetPemalready performs that encoding. The PR version would fail those assertions and should not replace the newer implementation.sceConvertKeycodeGetCharacterFromKeyboardDatais changed from a no-argument stub to a constant error return, but currentmainalready has a five-argument implementation that reads the requested keyboard data. Keep the current implementation after syncing.sceHttpSetCookieEnabled(..., 1)now returns success without enabling cookies;docs/dev/debt/libSceHttp.mdsays enabling throws because the offline backend has no cookie engine. Please retain the throw until that behavior exists.
Current main merge-tree reports conflicts in libSceConvertKeycode/Export.cpp and libSceSsl/Export.cpp. python3 tools/check_conventions.py --base upstream/main and git diff --check pass, but the PR description gives no focused test command/results. Please rebase onto current main, preserve its existing implementations, keep unsupported cookie enabling explicit, and rerun the focused keycode/SSL tests.
MarcZX10
left a comment
There was a problem hiding this comment.
This changes sceHttpSetCookieEnabled(id, 1) from the required unimplemented-path throw to success, but the option is not stored or applied anywhere in this diff. A caller is told cookies are enabled when the implementation still has no cookie support; this directly violates CONTRIBUTING.md’s every-function contract and its limited silent-stub exception. Keep the unsupported case throwing (and retain a test for it) until cookie behavior is implemented. The overlap bot also could not compare libSceHttp/Export.cpp against #9834; both diffs modify this exact export, and #9826 does too. Please coordinate merge order with those authors and declare Depends on where needed.
MarcZX10
left a comment
There was a problem hiding this comment.
Two blocking issues in the changed behavior:
sceSslGetPemcopiescrt->rawverbatim. In this backend, certificate inputs are parsed into Mbed TLS X.509 objects andrawis the DER certificate buffer (see the bundledmbedtls_x509_crt_parse_derpath); this does not produce PEM text. Also, when a non-null output buffer is too short, the function currently reports success after only updating*pemLen. Please serialize the DER certificate to PEM and match the established required-size/short-buffer behavior (for example, comparesceSslGetSerialNumber), with focused tests for sizing, short buffers, and returned PEM content.sceHttpSetCookieEnabled(id, 1)now returns success but this PR adds no cookie storage or handling.AGENTS.mdsays never return made-up success. Keep the unsupported enable path throwing until cookie behavior exists.
The cookie setter change is also duplicated in open #9826; the overlap report and checklist need an agreed order/dependency rather than Depends on: none. The branch is based on 11f976b2, 44 commits behind current main a6646f84. The report only identifies a Windows build, and current product jobs are skipped; focused behavior tests are needed on the final rebased head. I did not run local tests for these changes.
…Http, libSceSsl and libSceConvertKeycode Took only sceConvertKeycodeGetCharacterFromKeyboardData (returns INVALID_ADDRESS, our parameter list kept) and listed it as a silent stub; kept our fuller sceSslGetPem (PEM encoding) and sceHttpSetCookieEnabled (option stored).
|
Closing, with reasons so you can reopen a fixed version:
Welcome to reopen with one function per PR, a reference for each behaviour, and a test that fails without the change. |
What
Complete stubs in libSceHttp, libSceSsl and libSceConvertKeycode.
Tested
Windows 10 x64, net and input stubs.
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)