Conversation
…info walk CodeQL raised rust/access-invalid-pointer on the `&*ai` dereference in resolve_address. That alert is a false positive. The query's sources are models-as-data entries tagged `pointer-invalidate` -- freeing functions -- and the only one here was the `libc::freeaddrinfo` inside the AddrInfoList Drop impl added in fe25dc1. The query has no model of Rust drop order, so it cannot see that the guard drops at end of scope, strictly after the loop and after `addr` has been copied out. No dereference ever touched freed memory. Rewriting it anyway, because the query's own recommendation -- use safe Rust types -- is available here and is simply better code. The function only ever mapped a name to its first IPv4 address, which is what ToSocketAddrs does. That deletes the unsafe block, the hand-rolled free guard, the read_unaligned needed because libc's buffer carries no alignment guarantee for sockaddr_in, and five libc imports, and it retires the alert rather than requiring a dismissal. Behaviour is preserved. The old hints set AI_CANONNAME, but the canonical name was never read. The old loop used Ipv4Addr::UNSPECIFIED as its not-found sentinel, so a host resolving to 0.0.0.0 produced an error; that rejection is kept, now stated outright, since the wildcard address names no peer to send to. IPv6 results are still skipped because the talk control message carries an Osockaddr, which is IPv4-only. Tests were written against the old implementation first and passed unchanged against the new one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dvoJ2nMCQHSN8h9yJuVG
test_diff_recursive_symlink_cycle_terminates created the symlink that forms the cycle, then checked the result with `assert!(linked || true, ...)`, which is true regardless. clippy::overly_complex_bool_expr is denied by default, so this failed `cargo clippy --all-targets` for the whole workspace and masked a second failure in posixutils-process behind it. The assert was not merely redundant. The symlink is the entire subject of the test: without it, `diff -r` has no cycle to walk into and the comparison below passes while proving nothing. Any failure to create it therefore has to fail the test, not be waved through, so it is now an .expect() matching the mkdir and write above it. Both supported platforms symlink in the temp directory, so the only realistic way this fails is a directory left behind by a crashed earlier run, since the name is keyed on a process ID and those are recycled. Removing the base first makes the fixture idempotent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dvoJ2nMCQHSN8h9yJuVG
Two clippy failures on non-Linux hosts, both previously hidden behind the diff-tests failure that stopped the workspace lint early. wait_for_open_fd polled /proc in a deadline loop, but the cfg(not(linux)) arm of the body slept once and returned unconditionally, so the loop could never reach a second iteration: clippy::never_loop, denied by default, plus an unreachable statement warning for the sleep after it. The loop now exists only on Linux, where there is a /proc to re-check; elsewhere the one-shot sleep stands on its own, which is what the code already did. Every test in the `modes` module is cfg(target_os = "linux"), as are the only callers of parse_posix_pid_list, so on other platforms both compiled to nothing but dead-code warnings -- three of them, for the module's two unused imports and its unused a_mount_point helper. Gating them the way `tcp`, `udp`, and `unix` are already gated is a no-op on Linux, since it is the same condition that already gates every item inside. Workspace `cargo clippy --release --all-targets` is now clean. The Linux configuration is unchanged by construction but was not compiled here; this host has no Linux target installed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01R9dvoJ2nMCQHSN8h9yJuVG
There was a problem hiding this comment.
Pull request overview
This pull request updates several utilities’ test and runtime plumbing, primarily by simplifying talk’s IPv4 address resolution (removing a manual getaddrinfo walk) and tightening up test behavior in diff and fuser.
Changes:
talk: replace the manualgetaddrinfo/freeaddrinfoiteration withstd::net::ToSocketAddrs, while explicitly selecting a usable IPv4 address, and add focused unit tests for the new resolver behavior.difftests: make the symlink-cycle regression test deterministic by requiring symlink creation, and pre-clean the temp directory to avoid PID-reuse leftovers.fusertests: improve platform gating to avoid non-Linux dead-code warnings, and adjust the/procpolling helper structure for cleaner cfg-specific compilation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| users/talk.rs | Switches IPv4 address resolution to ToSocketAddrs (no manual addrinfo management) and adds resolver unit tests. |
| text/tests/diff-tests.rs | Hardens the recursive symlink-cycle test by enforcing symlink creation and cleaning stale temp dirs before setup. |
| process/tests/fuser/mod.rs | Adds Linux gating for the modes module and the POSIX PID-list parser helper to avoid dead-code warnings off Linux. |
| process/tests/fuser/basic.rs | Refactors the /proc-polling wait helper to avoid cfg-related unused imports/logic while preserving behavior. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.