Skip to content

feat(libc): implement IPv4 address helpers - #2231

Closed
MarcZX10 wants to merge 5 commits into
boykopovar:mainfrom
MarcZX10:codex/libc-inet-address
Closed

MarcZX10 wants to merge 5 commits into
boykopovar:mainfrom
MarcZX10:codex/libc-inet-address

Conversation

@MarcZX10

@MarcZX10 MarcZX10 commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What

Implement the guest libc IPv4 address helpers used by the current homebrew audit:

  • __inet_aton parses the FreeBSD one-to-four-part IPv4 forms, including octal and hexadecimal components, and leaves the output untouched on invalid input.
  • __inet_addr returns the parsed address in network byte order or INADDR_NONE, preserving FreeBSD's one-part unsigned wrap behavior.
  • __inet_ntoa formats the address bytes in network order into a per-thread buffer.

The tests cover dotted and abbreviated forms, octal and hexadecimal forms, whitespace termination, invalid input/output preservation, FreeBSD wrap cases, buffer reuse, and round trips across all three functions.

This PR consolidates the work in #1848, #1972, and #2208 into the previously reviewed IPv4-helper change from #1766. The original authors' commits are preserved in this branch history. Those three PRs currently overlap this implementation; this PR is intended as the single canonical change after the current-head Build is reviewed.

Tested

Linux x86-64, GCC 12.2, Release:

  • guest_strings passed (1/1) on the final source tree.
  • tools/check_conventions.py --base upstream/main passed.
  • git diff --check upstream/main...HEAD passed.
  • The libc and libs targets plus a full all-target build completed. GCC 12.2 cannot compile the unrelated std::chrono::clock_cast in current main, so the local full-build run temporarily applied the equivalent file-clock conversion from fix(SaveData): use C++20 file clock conversion #1260; that unrelated source edit was restored before the final focused test and is not part of this PR.

A full upstream Build on this head is still needed. No PS5 hardware or title was run locally.

Additional title stop evidence (not run with this PR)

A contributor reported that RetroArch (PPSA99169) and Porpoise (PPSA99764) both stop on __inet_addr on current main, then stop on __inet_ntoa after adding only __inet_addr. The report used Linux title runs with an __inet_addr implementation and did not run this PR; it supports the imports' relevance but does not establish title compatibility. See the report.

Native Windows evidence on pre-rebase head 6923a54e

A Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release run passed guest_strings with no skipped cases. The aggregate CTest run passed 519/519, but that build included 38 other PRs and needed local fixes to unrelated #1611, #1796, and #1824. This is not an unmodified upstream Build for this PR. No title was run.

Current head after rebase

Current head 863ff49fed19b11b21d4c3974ef09d3bc3e14b47 is based on current upstream main f8c2f072463465f875dedb0d20b66c339160c1a1.

  • Linux x86-64, GCC 12.2, Release: rebuilt guest_string_tests; guest_strings passed (1/1).
  • python3 tools/check_conventions.py --base upstream/main and git diff --check upstream/main...HEAD pass.
  • The exact-head upstream Linux/Windows Build and relinker matrix is still required. The prior Windows results above are from before the rebase.

Checklist

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

The full local Linux build completed with the GCC 12 compatibility workaround documented above, and the focused guest_strings CTest and Conventions pass on the final source tree. Under CONTRIBUTING, the full Linux/Windows Build and relinker matrix needs an authorized collaborator. Could a collaborator run Actions → Build for PR #2231 so this exact head is checked before the overlapping PRs are closed?

@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
#2208 __inet_ntoa_nid_postfix

@github-actions github-actions Bot added the libc label Oct 9, 2026
@CouchRiv

CouchRiv commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

On the "no title stop for __inet_ntoa" point: two catalog titles stop on it once __inet_addr exists. Linux (CachyOS, GCC, Release), titles relinked and run in a bubblewrap sandbox, main f145139 plus an __inet_addr implementation (our closed #1974, same behaviour as #1972):

Title main with __inet_addr
RetroArch PPSA99169 1.0.0-beta.1 undefined symbol: 7p7kTAJcuGg (__inet_addr) undefined symbol: 6i5aLrxRhG0 (__inet_ntoa)
Porpoise PPSA99764 2.7 undefined symbol: 7p7kTAJcuGg undefined symbol: 6i5aLrxRhG0

Not run with this PR.

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for running the full Linux/Windows Build workflow for #2208 and confirming your __inet_ntoa commit is preserved in this PR. Would you be able to run the same fork workflow against #2231’s exact current head 6923a54e? Your #2208 run is strong evidence for the shared commit, but this head also integrates the other IPv4 helpers and round-trip tests, so an exact-head run would close the remaining Windows/full-suite gap while the upstream manual Build is pending.

@popcoder565

Copy link
Copy Markdown
Contributor

Native Windows run, since no Windows results were posted. Setup: Windows 10 (19045), MinGW-w64 GCC 15.2.0 (WinLibs r7), Release. Head 6923a54e, merged onto main d70b899 in one build with 38 other PRs that had no Windows run.

No title was run.

🤖 Addressed by Claude Code

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for running the exact 6923a54e head and recording the Windows results. I added the focused guest_strings pass and the 519/519 batch CTest result to the PR description, with the three unrelated local Windows fixes clearly called out. The full upstream Build and title compatibility are still outstanding, so I have left those limitations explicit.

LeandroLP23 and others added 5 commits October 9, 2026 17:41
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>
Format the address bytes in network order as dotted decimal into a
per-thread buffer, like FreeBSD's inet_ntoa returns its static one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MarcZX10
MarcZX10 force-pushed the codex/libc-inet-address branch from 6923a54 to 863ff49 Compare October 9, 2026 17:41
@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Update: #2231 is now rebased onto current main (f8c2f072) at head 863ff49f. The diff remains limited to StringExtensions.cpp and GuestStrings.cpp; Linux guest_strings passes 1/1 and Conventions passes. The Windows 519/519 aggregate evidence was on the pre-rebase head and is labeled that way in the description. Exact-head Windows and the required upstream Build matrix remain outstanding; could an authorized collaborator run the Build on this head?

@MarcZX10

MarcZX10 commented Oct 9, 2026

Copy link
Copy Markdown
Contributor Author

Maintainer decision needed

The exact-head Conventions run 37968104485 fails because the checklist still reports open overlapping PRs. The successful run on the same head does not clear that finding.

The authors of #1848 and #1972 have said they prefer to keep their PRs open until a maintainer chooses which implementation to land. #2208 is also still open. The current #2231 branch contains the three original implementation commits with their authorship, plus round-trip tests. I recommend keeping #2231 as the canonical combined change.

The rebased head 863ff49f is based on current main f8c2f072; Linux guest_strings passes. The 519/519 Windows CTest report is from pre-rebase head 6923a54e, and the full upstream Build/relinker run on the current head is still missing.

Could a maintainer decide whether to keep #2231 or the three individual PRs? After that choice, the overlapping branches and checklist can be updated accurately, then the exact head can receive its required Build run.

@oneandonlydean

Copy link
Copy Markdown
Collaborator

Closing in favour of #1848, #1972 and #2208: each adds one of these functions with a title stop, and they'll go in in that order. The round-trip test can come as a follow-up once they're in.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants