Audit MINOR #37: managed ARP (GetIpNetTable) + Ping rewrite - #78
Merged
Conversation
….Net Ping Replaces the shelled `arp -a` (locale-fragile regex parse) and `ping` with managed APIs: - New IArpTableProvider abstraction. WindowsArpTableProvider reads the ARP cache via iphlpapi GetIpNetTable (P/Invoke); ShellArpTableProvider keeps `arp -a` for Linux/fallback. Registered by OS in AddControlMenuServices. - PingAsync uses System.Net.NetworkInformation.Ping (cross-platform, no process spawn); honours cancellation, maps non-Success/exception to unreachable. - NetworkDiscoveryService now depends on IArpTableProvider, not ICommandExecutor. Tests: pure ARP-row mapping (MapRows), arp -a parse (both layouts), provider exit handling, a real GetIpNetTable marshal smoke (Windows-gated), and managed-ping loopback/unroutable. 633 green.
…ows-gate ping tests Whole-branch review follow-ups (both Important, reliability): - WindowsArpTableProvider.Read now retries when GetIpNetTable returns ERROR_INSUFFICIENT_BUFFER (the ARP table can grow between the sizing call and the fetch), re-reading the bumped size — was silently returning an empty table on a busy host. ParseTable extracted; padding note added. - The two real-ICMP ping tests are gated to Windows (the app's platform; loopback ICMP can be blocked on a locked-down Linux runner). Doc wording: "Linux" -> "non-Windows".
bilbospocketses
enabled auto-merge (squash)
June 24, 2026 20:52
bilbospocketses
added a commit
that referenced
this pull request
Jun 24, 2026
) (#81) The wrap-up doc sweep caught four stale TECHNICAL_GUIDE references the feature PRs missed: - NetworkDiscoveryService delegates to IArpTableProvider (Windows GetIpNetTable P/Invoke + arp -a fallback); PingAsync is managed System.Net.NetworkInformation.Ping (#37 / PR #78) - GetDefaultBackupDirectory is pure — the backup-write path creates the dir, not the getter (#40 / PR #77) - Expand-Cm7z extracts .7z via the bundled SharpCompress (tools/Cm7zExtract), no vendored 7zr (#46 / PR #80) - in-app update apply signals via a shared UpdateApplyState flag + explicit Program return, not a clobberable Environment.ExitCode (#50 / PR #79)
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
1 participant
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.
PR 3 of the audit MINOR mop-up — finding #37, the managed network rewrite (the full-managed option).
Replaces the shelled
arp -a(locale-fragile regex parsing) andpingwith managed APIs:IArpTableProviderabstraction. On Windows,WindowsArpTableProviderreads the ARP cache via the IP Helper API (GetIpNetTable) — noarp.exespawn, no text scraping. On other platformsShellArpTableProviderkeepsarp -a(no managed ARP API there). Registered by OS inAddControlMenuServices.PingAsyncusesSystem.Net.NetworkInformation.Pinginstead of spawningping— cross-platform, honours cancellation.NetworkDiscoveryServicenow depends onIArpTableProvider.Tests: pure ARP-row mapping (
MapRows),arp -aparse for both layouts, provider exit handling, a Windows-gated realGetIpNetTablemarshal smoke, and managed-ping loopback/unroutable checks. Full suite 633 green; the real P/Invoke returns this machine's ARP table without throwing.Independent review verified the P/Invoke marshaling (struct layout, offset, byte-order, memory safety) against the Win32 docs and came back clean. Its two reliability notes are addressed: a buffer-growth retry on
GetIpNetTable(the table can grow between the size and fetch calls), and gating the real-ICMP ping tests to Windows.Plan:
docs/superpowers/plans/2026-06-24-audit-minor-mopup.md(PR 3 of 5).