Repository navigation
fix(libkernel): getsockopt reads options of libSceNet sockets - #9515
EagleBearerYS wants to merge 6 commits into
Conversation
…ockopt sceNetSetsockopt takes both, but sceNetGetsockopt failed them with EOPNOTSUPP. It now reads them from the host socket. FreeBSD's sogetopt returns so_options & option for the boolean options, so they read back as 0x4 and 0x20 when set.
setsockopt reaches a socket created with sceNetSocket through the option setter libSceNet registers, but getsockopt looked the descriptor up only among libkernel's own sockets and failed, so a title could set an option on such a socket through the BSD call and not read it back. libSceNet now registers an option getter with libkernel next to its setter, which answers through sceNetGetsockopt, so SO_RCVTIMEO and SO_SNDTIMEO come back as a struct timeval.
MarcZX10
left a comment
There was a problem hiding this comment.
The socket-hook change and the reported Windows/Linux tests look promising, but the checklist currently says Depends on: none while the PR description identifies two live overlaps. CONTRIBUTING.md requires overlap authors to agree on merge order and declare the dependency. The getsockopt_nid_postfix hunk overlaps open #7511, which validates pointer/length ordering in the same function; this PR inserts the libSceNet dispatch before that function’s current lookup. The docs/dev/debt/libkernel.md hunk overlaps open #9365 on the same socket-call sentence. Please coordinate with both authors, identify the agreed order, and update the checklist (and #9365’s “no open pull request edits these sentences” statement) before merge. I am not asking for a code change based on the reviewed diff.
|
Thanks, @MarcZX10. This PR goes after both: Depends on now lists #7511 and #9365, and I've left a note on each. When it rebases on #7511, the libSceNet getter call moves after #7511's pointer and length checks, so a libSceNet socket fails those the same way libkernel's own sockets do. On #9365, this PR's addition of |
|
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.
Re-reviewed the updated full body and diff after the author recorded the shared getsockopt/debt ordering with #7511 and #9365. The new getter registration is adjacent to the existing setter bridge, and the reported tests exercise the net-specific and libkernel dispatch paths with the FreeBSD timeval representation. Current hosted checks and the reported cross-platform test suites support the change; I did not run a local build. The prior overlap coordination blocker is resolved.
MarcZX10
left a comment
There was a problem hiding this comment.
Rechecked the current head and the updated dependency coordination. The PR now declares #7511 and #9365, explains how the getter dispatch follows #7511 pointer/length checks, and records the shared debt sentence order with #9365. The tests cover libSceNet boolean reads and libkernel timeval dispatch; author reports repeated Linux/Windows red/green runs and broader suites, with one Linux timing test failing on both base and change. Hosted eligibility, conventions, intake, labels and report checks pass; product/relinker jobs are skipped. I did not run local tests.
|
Linux data point for
|
|
@xXPBSXx thanks for the Linux run, both red and green. It's in Tested now. |
|
@EagleBearerYS I found a partial overlap with my #9542 while checking the wider socket-option queue: both add native SO_REUSEADDR/SO_BROADCAST readback. Mine also handles short output buffers and has a dedicated regression. #9578 carries your readback commit as well. Would #9523 → #9542 → this PR → #9578 work for you, keeping your existing #7511/#9365 prerequisites? That would let this PR keep the getter forwarding and its tests while dropping the duplicate readback hunk. If yours needs to land first, I can instead rebase #9542 and retain only its short-buffer behavior and dedicated test. I have marked mine as a draft while we agree on the order. |
|
@Queuse that order works: #9523 → #9542 → this PR → #9578, with #7511 and #9365 still before this PR. #9542's readback is the same as this PR's e887616 (the host's |
|
Thanks, @EagleBearerYS. I've recorded the agreed #9523 → #9542 → #9515 → #9578 order and your combined test results in #9542, keeping your #7511/#9365 prerequisites explicit. The overlap item in the checklist is now settled. #9542 is still a draft while the separate test-reorganization coordination with #6044 is pending. |
# Conflicts: # docs/dev/debt/libkernel.md
|
@EagleBearerYS This PR now conflicts with main in |
Main's boykopovar#10706 declares getsockopt_nid_postfix and sceNetGetsockopt in the libraries' public headers, so this branch's local declarations of them in GuestSceNet.cpp and libSceNet's Export.cpp go.
|
@oneandonlydean Thanks, merged |
Main extends the libkernel debt entry on libSceNet sockets with poll and fcntl, where this branch adds getsockopt to the same entry; the merge keeps both.
|
The #9523/#9542 branches are refreshed, and #9542 is ready for review. The agreed #9523 → #9542 → #9515 → #9578 order still works on my side. #6044 was closed without the test relocation, so the draft blocker mentioned in my earlier reply is gone. I preserved main's merged protocol/TCP_NODELAY getters and their short-output rules in both refreshes; each branch passes its own regression plus the protocol, TCP_NODELAY and buffer controls on Windows. |
Title-less conformance fixes of this kind are merged now, for example #7365 and #8411.
What
setsockoptreaches a socket created withsceNetSocketthrough the option setter libSceNet registers with libkernel, butgetsockoptlooks the descriptor up only among libkernel's own sockets and fails. A title can set an option on such a socket through the BSD call and not read it back.getsockoptasks it for a descriptor libkernel doesn't own, assetsockoptdoes. The getter answers throughsceNetGetsockopt, soSO_RCVTIMEOandSO_SNDTIMEOcome back as astruct timeval, as FreeBSD 11.4'ssogetoptreturns them.sceNetGetsockoptreads backSO_REUSEADDRandSO_BROADCAST, whichsceNetSetsockopttakes butsceNetGetsockoptfailed withEOPNOTSUPP. It reads them from the host socket. FreeBSD'ssogetoptreturnsso_options & optionfor the boolean options, so they read back as 0x4 and 0x20 when set.getsockoptwithsetsockopt.Two commits: e887616 for
sceNetGetsockopt, aea804d for the libkernel getter and the debt entry. No title is claimed.Overlap, merge order only: this PR goes after #7511 and #9365, and rebases on them. #7511 adds pointer and length checks at the start of
getsockopt_nid_postfix, where this PR adds the getter call; after the rebase the call comes after those checks, so a libSceNet socket fails a bad pointer or length as libkernel's own sockets do. #9365 corrects the samecloseandsetsockoptdebt sentence, and this PR's addition ofgetsockoptto it goes on top. #9542 (Queuse) adds the sameSO_REUSEADDRandSO_BROADCASTreadback insceNetGetsockopt, with short-buffer handling. The order is agreed with Queuse in this thread (comment): #9523 → #9542 → this PR → #9578. The two merge as text, but would put the same twocaselabels in one switch, so this PR drops e887616'ssceNetGetsockopthunk at its first merge ofmainafter #9542 lands. It keeps the libkernel getter forwarding and its tests. On #9542 with this PR merged in and that hunk dropped,guest_sce_netandguest_net_boolean_readbackpass on both runners (run).Tested
On GitHub's runners from my fork, on
mainat a6dcea9 (run, of the same code; aea804d adds the debt edit only):guest_sce_net, on a datagram socket made withsceNetSocket:sceNetGetsockoptreadsSO_BROADCASTandSO_REUSEADDRback as 0x20 and 0x4 aftersceNetSetsockoptsets them;getsockoptreadsSO_BROADCASTback as 0x20;setsockoptsetsSO_RCVTIMEOto 0.25 s, libkernel'sgetsockoptreturns the samestruct timeval.main's codeguest_sce_netfails on Linux and Windows. With the change it passes 10 runs in a row on both.core/libs/prx/libcandcore/libs/prx/libkernelpasses: 142 and 207 on Windows, 197 libkernel tests on Linux. In the Linux libc suite (115), the timing testprecise_waitfailed with and without the change in this run.tools/check_conventions.py --base origin/mainpasses.guest_sce_netpasses 3 of 3, and with libSceNet'sExport.cppand libkernel'sSocketPoll.hpp,Socket.cppandUdp.cppat the merge base and the PR's tests kept, it aborts.main938812d. The only conflict wasdocs/dev/debt/libkernel.md, wheremainreworded thesceKernelPreadline next to this PR'scloseandgetsockoptline; both changes are kept, and the PR's own diff is unchanged. On GitHub's runners (run),guest_sce_net,guest_socket_options,guest_socket_errorandguest_socket_descriptor_flagspass 10 runs in a row on Linux and on Windows, 4 of 4, none skipped. The libc and libkernel suites pass on both (119 and 201 tests on Linux, 146 and 211 on Windows), run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.main67bbe03. The only conflict wasdocs/dev/debt/libkernel.md, where feat(libkernel): answer sceKernelAddReadEvent and sceKernelAddWriteEvent on regular files #9099 (aarushkha, regular-file events) reworded thesceKernelAddReadEventpipe entry and added its own regular-file entry just above this PR'sclose,setsockoptandgetsockoptline; the merge keeps both ofmain's entries and this PR's line, and the PR's own diff is unchanged. On GitHub's runners (run),guest_sce_net,guest_socket_options,guest_socket_errorandguest_socket_descriptor_flagspass 10 runs in a row with the change, on Linux and on Windows: 4 run, 4 pass, 0 skip, 0 fail. Withmain's code,guest_sce_netfails on both. The libc and libkernel suites pass on both (143 and 205 tests on Linux, 170 and 218 on Windows), run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.maina512780. The one conflict was incore/libs/tests/GuestSceNet.cpp, where refactor(libs): add full export headers for libs #10706 (the public-header migration) removed the test's local declarations, among them this PR'sgetsockopt_nid_postfix. The merge takesmain's side, since libkernel'sinclude/Api.hppnow declaresgetsockopt_nid_postfixwith the same arguments. It also drops this PR's localsceNetGetsockoptdeclaration in libSceNet'sExport.cpp, whichtests/test_public_headers.py --audit-onlyreports as a second declaration next toNet.hpp's. So the PR's own diff loses those two declarations and is otherwise unchanged. On GitHub's runners (run),guest_sce_net,guest_socket_options,guest_socket_error,guest_socket_descriptor_flagsandpublic_headerspass 10 runs in a row with the change, on Linux and on Windows: 5 run, 5 pass, 0 skip, 0 fail. Withmain's code,guest_sce_netfails on both. The libc and libkernel suites pass on both (143 and 206 tests on Linux, 170 and 222 on Windows), run withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.main66e6772 (run). The one conflict wasdocs/dev/debt/libkernel.md's entry on libSceNet sockets, wheremainaddspollandfcntland this PR addsgetsockopt; the merge keeps both, in the same words as feat(libSceNet): implement the Ex and Internal socket forms, sceNetSysctl, scope-ID text and the default route #9578's merge. The PR's code diff is unchanged.guest_sce_net,guest_socket_descriptor_flags,guest_socket_error,guest_socket_optionsandpublic_headerspass 10 runs in a row. Linux: 5 run, 5 pass, 0 skip, 0 fail. Windows: 5 run, 5 pass, 0 skip, 0 fail. The before arm failsguest_sce_neton both. Suites: libc and libkernel pass on Linux (144 and 207) and on Windows (171 and 223). The suites ran withAPS5_SKIP_TIMING_TESTS=1, which relaxesprecise_wait's median bounds on GitHub's hosted runners.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)