refactor: share iter_nameservers helper and reuse resolv.conf content

Post-review simplifications on the stale-backup fix:

- Extract iter_nameservers(&str) helper used by both parse_resolv_conf
  and resolv_conf_has_real_upstream. Eliminates the duplicated
  line-by-line nameserver parsing (findings from reuse review).
- install_linux: reuse the already-read resolv.conf content via
  std::fs::write instead of a second read via std::fs::copy.
- install_macos / install_windows: flatten the conditional eprintln
  pattern — always print a blank line, conditionally print the save
  message. Equivalent output, less branching.

Net −12 lines. All 130 tests still pass, clippy clean.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Razvan Dimescu
2026-04-08 04:33:34 +03:00
parent 08291c4ace
commit 68c7fc4130

View File

@@ -214,7 +214,18 @@ fn discover_linux() -> SystemDnsInfo {
} }
} }
/// Parse resolv.conf in a single pass, extracting both the first non-loopback /// Yield each `nameserver` address from resolv.conf content. No filtering —
/// callers decide what counts as a real upstream.
#[cfg(any(target_os = "linux", test))]
fn iter_nameservers(content: &str) -> impl Iterator<Item = &str> {
content.lines().filter_map(|line| {
let mut parts = line.trim().split_whitespace();
(parts.next() == Some("nameserver")).then_some(())?;
parts.next()
})
}
/// Parse resolv.conf in a single pass, extracting the first non-loopback
/// nameserver and all search domains. /// nameserver and all search domains.
#[cfg(target_os = "linux")] #[cfg(target_os = "linux")]
fn parse_resolv_conf(path: &str) -> (Option<String>, Vec<String>) { fn parse_resolv_conf(path: &str) -> (Option<String>, Vec<String>) {
@@ -222,19 +233,13 @@ fn parse_resolv_conf(path: &str) -> (Option<String>, Vec<String>) {
Ok(t) => t, Ok(t) => t,
Err(_) => return (None, Vec::new()), Err(_) => return (None, Vec::new()),
}; };
let mut upstream = None; let upstream = iter_nameservers(&text)
.find(|ns| !is_loopback_or_stub(ns))
.map(str::to_string);
let mut search_domains = Vec::new(); let mut search_domains = Vec::new();
for line in text.lines() { for line in text.lines() {
let line = line.trim(); let line = line.trim();
if line.starts_with("nameserver") { if line.starts_with("search") || line.starts_with("domain") {
if upstream.is_none() {
if let Some(ns) = line.split_whitespace().nth(1) {
if !is_loopback_or_stub(ns) {
upstream = Some(ns.to_string());
}
}
}
} else if line.starts_with("search") || line.starts_with("domain") {
for domain in line.split_whitespace().skip(1) { for domain in line.split_whitespace().skip(1) {
search_domains.push(domain.to_string()); search_domains.push(domain.to_string());
} }
@@ -248,27 +253,14 @@ fn parse_resolv_conf(path: &str) -> (Option<String>, Vec<String>) {
/// for a backup. /// for a backup.
#[cfg(any(target_os = "linux", test))] #[cfg(any(target_os = "linux", test))]
fn resolv_conf_is_numa_managed(content: &str) -> bool { fn resolv_conf_is_numa_managed(content: &str) -> bool {
if content.contains("Generated by Numa") { content.contains("Generated by Numa") || !resolv_conf_has_real_upstream(content)
return true;
}
!resolv_conf_has_real_upstream(content)
} }
/// True if the resolv.conf content has at least one non-loopback, non-stub /// True if the resolv.conf content has at least one non-loopback, non-stub
/// nameserver. An all-loopback resolv.conf is self-referential. /// nameserver. An all-loopback resolv.conf is self-referential.
#[cfg(any(target_os = "linux", test))] #[cfg(any(target_os = "linux", test))]
fn resolv_conf_has_real_upstream(content: &str) -> bool { fn resolv_conf_has_real_upstream(content: &str) -> bool {
for line in content.lines() { iter_nameservers(content).any(|ns| !is_loopback_or_stub(ns))
let line = line.trim();
if let Some(rest) = line.strip_prefix("nameserver") {
if let Some(ns) = rest.split_whitespace().next() {
if !is_loopback_or_stub(ns) {
return true;
}
}
}
}
false
} }
/// Query resolvectl for the real upstream DNS server (e.g. VPC resolver on AWS). /// Query resolvectl for the real upstream DNS server (e.g. VPC resolver on AWS).
@@ -629,10 +621,9 @@ fn install_windows() -> Result<(), String> {
let needs_reboot = disable_dnscache()?; let needs_reboot = disable_dnscache()?;
register_autostart(); register_autostart();
if has_useful_existing {
eprintln!(); eprintln!();
} else { if !has_useful_existing {
eprintln!("\n Original DNS saved to {}", path.display()); eprintln!(" Original DNS saved to {}", path.display());
} }
eprintln!(" Run 'numa uninstall' to restore.\n"); eprintln!(" Run 'numa uninstall' to restore.\n");
if needs_reboot { if needs_reboot {
@@ -891,10 +882,9 @@ fn install_macos() -> Result<(), String> {
.status(); .status();
} }
if has_useful_existing {
eprintln!(); eprintln!();
} else { if !has_useful_existing {
eprintln!("\n Original DNS saved to {}", backup_path().display()); eprintln!(" Original DNS saved to {}", backup_path().display());
} }
eprintln!(" Run 'sudo numa uninstall' to restore.\n"); eprintln!(" Run 'sudo numa uninstall' to restore.\n");
@@ -1253,12 +1243,10 @@ fn install_linux() -> Result<(), String> {
); );
} else if current_is_numa_managed { } else if current_is_numa_managed {
eprintln!(" warning: /etc/resolv.conf is already numa-managed; no fresh backup written"); eprintln!(" warning: /etc/resolv.conf is already numa-managed; no fresh backup written");
} else { } else if let Some(content) = current.as_deref() {
match std::fs::copy(resolv, &backup) { std::fs::write(&backup, content)
Ok(_) => eprintln!(" Saved /etc/resolv.conf to {}", backup.display()), .map_err(|e| format!("failed to backup /etc/resolv.conf: {}", e))?;
Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} eprintln!(" Saved /etc/resolv.conf to {}", backup.display());
Err(e) => return Err(format!("failed to backup /etc/resolv.conf: {}", e)),
}
} }
if resolv if resolv