Repository navigation
fix(libkernel): report NODEV and a per-socket inode from fstat on a socketpair end - #10621
Conversation
MarcZX10
left a comment
There was a problem hiding this comment.
The GuestSockets::Stat change and tests look consistent with the described AF_UNIX behavior: inode assignment occurs under socketsMutex, the Socket owns the value so dup shares it, and the AF_INET control preserves zeros. One coordination item remains in the PR body: it says #8344 adds a Socket member adjacent to this new inode field and whichever merges second needs a rebase, but the checklist says Depends on: none. CONTRIBUTING.md asks overlapping PRs to agree on order and declare that dependency. Please record the agreed order (or state that #8344 is the prerequisite) so reviewers and automation can track the shared insertion hunk. I have not independently run the Windows socket tests; hosted product build legs were skipped by eligibility.
MarcZX10
left a comment
There was a problem hiding this comment.
The author has resolved the coordination item: #8344 is explicitly first, and this PR now declares Depends on: #8344 and says it will rebase and rerun guest_socket_pair after #8344 merges. I approve the reviewed implementation and tests at head c27cdcaf4cc1218a0679720ad6f3200645599255. The previously reviewed behavior remains consistent: inode assignment is protected by socketsMutex, lifetime is owned by the shared Socket (so dup retains identity), and AF_INET stats remain zero. Hosted eligibility/conventions/intake/labels pass; product checks were skipped. Windows socket test results are author-reported, not independently reproduced. The PR remains dependent on #8344 and should be revalidated after that rebase.
|
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.
|
…inode # Conflicts: # core/libs/tests/GuestSocketPair.cpp
|
@Retr0G28 thanks for the socket inode work. #10621 conflicts with main in two places. The Main added this sentence at the end of that entry: "A socketpair end reports st_dev and st_ino 0, where FreeBSD's uipc_sense reports NODEV and a per-socket number". Your PR now gives unix-family sockets a lazily assigned Could you merge main and restate that sentence to what still differs from FreeBSD once your PR is in? For example, whether |
…inode # Conflicts: # core/libs/prx/libkernel/Socket/src/Udp.cpp # docs/dev/debt/libkernel.md
|
@oneandonlydean Merged After this PR,
Head ada29c7, Windows 11 (GCC 15.2.0): |
What
fstaton a socketpair end reportedst_devandst_ino0, which the debt entry recorded. FreeBSD 11'suipc_sense, thepru_senseof a local socket, setsst_devtoNODEVandst_inoto the socket'sunp_ino. A socket gets itsunp_inofrom one global counter the first time it is sensed, skipping 0, and keeps it.dev_tandino_tare 32-bit in FreeBSD 11, as inFileStat, soNODEVis 0xFFFFFFFF. Inet sockets usepru_sense_null, which leaves both 0.GuestSockets::Statnow does the same for a socket of familyAF_UNIX, which onlysocketpaircreates. The socket gets a number from a counter undersocketsMutexthe first time it is stat'ed, and the number is kept inSocket. Adupshares theSocket, so it reports the same number, as it shares thestruct socketon FreeBSD. The other fields are unchanged.mainadded a sentence to thefstat on a guest socketdebt entry saying a socketpair end reportsst_devandst_ino0. It now names only what still differs: the number comes from a counter this process starts at 1, where FreeBSD'sunp_inois one counter for every local socket on the system, so the numbers differ from the console's.st_devnow matches.Test:
guest_socket_pairchecks that both ends of a stream pair reportst_dev0xFFFFFFFF and a nonzerost_ino. The two ends must differ, a secondfstatorsceKernelFstatmust give the same number, and adupof an end must report that end's number. AnAF_INETsocket still reportsst_devandst_ino0.Tested
Windows 11, WinLibs MinGW-w64 GCC 15.2.0 r7, CMake + Ninja, Release,
-DBUILD_TESTING=ON, rebased onmain6cd36e6.maina23f8fc: withmain'sUdp.cpp,guest_socket_pairfails (exit 0xc0000409 from the failedRequire). With this PR it passes, andctest -R "socket|sendfile|kqueue|poll"passes 39 of 39.ctest -R "socket|recvmsg|sendfile|kqueue|^guest_poll$"passes 32 of 32 when run serially.mainafc780e (head ada29c7): the same pattern passes 42 of 42 when run serially.python tools/check_conventions.py --base origin/mainreports no errors.AF_UNIXpair and the same code path runs. No title or homebrew run.Overlap (
tools/pr_overlap.py):peermember next to the newinodemember (Udp.cpp L124). It merged first; this branch keeps both members.fstat on a guest socketdebt entry, for the read bits after the peer closes, and code nearGuestSockets::Stat. The two touch different sentences and fields, so the second needs only a rebase of that entry.pollentry and stale socket statements. Neither changes thest_devorst_inosentence.GuestSockets::Stator the socketpairfstatchecks.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)