Repository navigation
feat(libc): implement __inet_aton - #1848
Conversation
PICO-8 for PS5 (PPSA99808) stops at start-up on the unresolved import a7ToDPsIQrc from libSceLibcInternal. The parser follows FreeBSD's inet_aton (lib/libc/inet/inet_addr.c): one to four parts in decimal, octal or hex, the last part filling the remaining bytes, input ending at a NUL or whitespace, and a null address only validating. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MarcZX10
left a comment
There was a problem hiding this comment.
The one-part limit is too large: limits[0] is UINT64_MAX, but the documented a form occupies 32 bits. As written, 4294967296 is accepted and truncated to 0.0.0.0. Also, the digit accumulation wraps in uint64_t: 18446744073709551617 wraps to 1 and is accepted as 0.0.0.1. Please cap the one-part form at 0xffffffff and reject overflow during multiply/add (or otherwise parse with checked bounds). Add boundary tests for 4294967295 (valid), 4294967296 (invalid), and a value that exceeds uint64_t; the same checked accumulation should cover hex and octal.
|
On the one-part limit: FreeBSD's I compared FreeBSD's parser (that file, built unchanged on Linux x86-64) against a parser written to the same rules for 24,806 inputs: part-boundary and 32/64-bit wrap values, every trailing byte, all whitespace characters, malformed prefixes. There were no differences. glibc rejects |
MarcZX10
left a comment
There was a problem hiding this comment.
Correction to my earlier review: I verified the pinned FreeBSD inet_aton source you linked. On amd64, the one-part form intentionally has no 32-bit range check, and accumulation in u_long wraps before htonl; the examples you gave match that implementation. I retract both the 32-bit cap request and the request to reject unsigned accumulation wrap. My earlier requested boundary tests would encode the wrong semantics. Sorry for the incorrect review. I cannot dismiss my prior review with this account; please treat this correction as withdrawing it.
MarcZX10
left a comment
There was a problem hiding this comment.
Approved after verifying the FreeBSD source semantics; my prior change request was incorrect and is withdrawn.
|
PR #2231 is now the canonical, more complete IPv4-helper change. It includes this implementation along with the other parsing/formatting helpers, preserves the original author commits in its history, and adds cross-function round-trip coverage. The exact head has a local Linux full-build result with the documented GCC 12 SaveData workaround, and I requested the upstream Build. Please close this overlapping PR as superseded once the current-head Build on #2231 is reviewed. |
|
Thanks for preserving the commit. I'd rather keep #1848 open until a maintainer decides: today #1839 was closed for putting three exports in one PR, and library functions go in when a title stops on them on main. #1848 is one export with PICO-8's stop on main (and your approval), while |
|
That concern is fair. For the combined IPv4-helper topic, I think #2231 is the better canonical PR: it preserves your |
|
Update after rebase: #2231 is now on current I still consider #2231 the better canonical target for the IPv4 helper family because it preserves and combines the three implementations and adds cross-function tests. The overlapping PRs remain open until maintainers decide whether this cohesive family should land together; I am not treating the old Windows batch as current-head validation. |
|
Checked against FreeBSD |
What
PICO-8 for PS5 (PPSA99808 v0.1.2, RafaelNGP/pico8-ps5, GPL-3.0) stops at start-up on
main:a7ToDPsIQrcis__inet_aton(tools/nid_names.py). It is the only title in @SleglJan's run of the catalog (#1578) that stops there.The parser follows FreeBSD's
inet_aton(lib/libc/inet/inet_addr.c):0) or hex (0x);a.b→a.0.0.bwithbup to 24 bits,a.b.cwithcup to 16 bits, a single number for all 32 bits);Windows has no
inet_aton, and glibc's differs in its overflow checks, so the parser is written out instead of calling the host's. It is inStringExtensions.cpprather than a new file, so it does not collide with theNetwork.cppthat #1766 (__inet_addr,__inet_ntoa) adds.Tested
Windows 11, MinGW-w64 GCC 15.2.0, Release,
main364a56b:+2thxYZ4sykfromlibkernel.prx(environ).guest_strings, 3/3, also after rebasing ontomain94b1eec. New checks:192.0.2.42,10.1.2,127.1,3232235777;08,1..2, letters, trailing garbage, a bare0x, a trailing dot, a sign.tools/check_conventions.pypasses.Linux and Windows: fork CI https://github.com/LeandroLP23/AnyPS5/actions/runs/37846914473, all jobs pass (on the same change based on
364a56b6).Checklist
main; no other open PR implements the same functionsNotImplemented_nid_no_patch); silent stubs are listed in TechnicalDebt.mdfiles or images added to the repository (attach them to this PR)