From 3f37ddbd22803a991ece4fa3fc78fda02e776697 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Sat, 28 Aug 2021 13:47:31 +0200 Subject: [PATCH] hostname: Cleanup - Attach context to I/O errors - Make flags override each other - Support invalid unicode as argument - Call WsaCleanup() even on panic - Do not use deprecated std::mem::uninitialized() --- .../workspace.wordlist.txt | 2 + src/uu/hostname/src/hostname.rs | 136 +++++++++--------- 2 files changed, 73 insertions(+), 65 deletions(-) diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index 29957fb12..d37a59465 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -68,6 +68,7 @@ structs substr splitn trunc +uninit # * uutils basenc @@ -277,6 +278,7 @@ ULONG ULONGLONG UNLEN WCHAR +WSADATA errhandlingapi fileapi handleapi diff --git a/src/uu/hostname/src/hostname.rs b/src/uu/hostname/src/hostname.rs index 8852e0f43..2de6627e8 100644 --- a/src/uu/hostname/src/hostname.rs +++ b/src/uu/hostname/src/hostname.rs @@ -10,18 +10,13 @@ #[macro_use] extern crate uucore; -use clap::{crate_version, App, Arg, ArgMatches}; use std::collections::hash_set::HashSet; use std::net::ToSocketAddrs; use std::str; -#[cfg(windows)] -use uucore::error::UUsageError; -use uucore::error::{UResult, USimpleError}; -#[cfg(windows)] -use winapi::shared::minwindef::MAKEWORD; -#[cfg(windows)] -use winapi::um::winsock2::{WSACleanup, WSAStartup}; +use clap::{crate_version, App, Arg, ArgMatches}; + +use uucore::error::{FromIo, UResult}; static ABOUT: &str = "Display or set the system's host name."; @@ -31,45 +26,52 @@ static OPT_FQDN: &str = "fqdn"; static OPT_SHORT: &str = "short"; static OPT_HOST: &str = "host"; -#[uucore_procs::gen_uumain] -pub fn uumain(args: impl uucore::Args) -> UResult<()> { - #![allow(clippy::let_and_return)] - #[cfg(windows)] - unsafe { - #[allow(deprecated)] - let mut data = std::mem::uninitialized(); - if WSAStartup(MAKEWORD(2, 2), &mut data as *mut _) != 0 { - return Err(UUsageError::new( - 1, - "Failed to start Winsock 2.2".to_string(), - )); +#[cfg(windows)] +mod wsa { + use std::io; + + use winapi::shared::minwindef::MAKEWORD; + use winapi::um::winsock2::{WSACleanup, WSAStartup, WSADATA}; + + pub(super) struct WsaHandle(()); + + pub(super) fn start() -> io::Result { + let err = unsafe { + let mut data = std::mem::MaybeUninit::::uninit(); + WSAStartup(MAKEWORD(2, 2), data.as_mut_ptr()) + }; + if err != 0 { + Err(io::Error::from_raw_os_error(err)) + } else { + Ok(WsaHandle(())) } } - let result = execute(args); - #[cfg(windows)] - unsafe { - WSACleanup(); + + impl Drop for WsaHandle { + fn drop(&mut self) { + unsafe { + // This possibly returns an error but we can't handle it + let _err = WSACleanup(); + } + } } - result } fn usage() -> String { format!("{0} [OPTION]... [HOSTNAME]", uucore::execution_phrase()) } -fn execute(args: impl uucore::Args) -> UResult<()> { +#[uucore_procs::gen_uumain] +pub fn uumain(args: impl uucore::Args) -> UResult<()> { let usage = usage(); let matches = uu_app().usage(&usage[..]).get_matches_from(args); - match matches.value_of(OPT_HOST) { + #[cfg(windows)] + let _handle = wsa::start().map_err_context(|| "failed to start Winsock".to_owned())?; + + match matches.value_of_os(OPT_HOST) { None => display_hostname(&matches), - Some(host) => { - if let Err(err) = hostname::set(host) { - return Err(USimpleError::new(1, format!("{}", err))); - } else { - Ok(()) - } - } + Some(host) => hostname::set(host).map_err_context(|| "failed to set hostname".to_owned()), } } @@ -81,64 +83,68 @@ pub fn uu_app() -> App<'static, 'static> { Arg::with_name(OPT_DOMAIN) .short("d") .long("domain") + .overrides_with_all(&[OPT_DOMAIN, OPT_IP_ADDRESS, OPT_FQDN, OPT_SHORT]) .help("Display the name of the DNS domain if possible"), ) .arg( Arg::with_name(OPT_IP_ADDRESS) .short("i") .long("ip-address") + .overrides_with_all(&[OPT_DOMAIN, OPT_IP_ADDRESS, OPT_FQDN, OPT_SHORT]) .help("Display the network address(es) of the host"), ) - // TODO: support --long .arg( Arg::with_name(OPT_FQDN) .short("f") .long("fqdn") + .overrides_with_all(&[OPT_DOMAIN, OPT_IP_ADDRESS, OPT_FQDN, OPT_SHORT]) .help("Display the FQDN (Fully Qualified Domain Name) (default)"), ) - .arg(Arg::with_name(OPT_SHORT).short("s").long("short").help( - "Display the short hostname (the portion before the first dot) if \ - possible", - )) + .arg( + Arg::with_name(OPT_SHORT) + .short("s") + .long("short") + .overrides_with_all(&[OPT_DOMAIN, OPT_IP_ADDRESS, OPT_FQDN, OPT_SHORT]) + .help("Display the short hostname (the portion before the first dot) if possible"), + ) .arg(Arg::with_name(OPT_HOST)) } fn display_hostname(matches: &ArgMatches) -> UResult<()> { - let hostname = hostname::get().unwrap().into_string().unwrap(); + let hostname = hostname::get() + .map_err_context(|| "failed to get hostname".to_owned())? + .to_string_lossy() + .into_owned(); if matches.is_present(OPT_IP_ADDRESS) { // XXX: to_socket_addrs needs hostname:port so append a dummy port and remove it later. // This was originally supposed to use std::net::lookup_host, but that seems to be // deprecated. Perhaps we should use the dns-lookup crate? let hostname = hostname + ":1"; - match hostname.to_socket_addrs() { - Ok(addresses) => { - let mut hashset = HashSet::new(); - let mut output = String::new(); - for addr in addresses { - // XXX: not sure why this is necessary... - if !hashset.contains(&addr) { - let mut ip = format!("{}", addr); - if ip.ends_with(":1") { - let len = ip.len(); - ip.truncate(len - 2); - } - output.push_str(&ip); - output.push(' '); - hashset.insert(addr); - } + let addresses = hostname + .to_socket_addrs() + .map_err_context(|| "failed to resolve socket addresses".to_owned())?; + let mut hashset = HashSet::new(); + let mut output = String::new(); + for addr in addresses { + // XXX: not sure why this is necessary... + if !hashset.contains(&addr) { + let mut ip = addr.to_string(); + if ip.ends_with(":1") { + let len = ip.len(); + ip.truncate(len - 2); } - let len = output.len(); - if len > 0 { - println!("{}", &output[0..len - 1]); - } - - Ok(()) - } - Err(f) => { - return Err(USimpleError::new(1, format!("{}", f))); + output.push_str(&ip); + output.push(' '); + hashset.insert(addr); } } + let len = output.len(); + if len > 0 { + println!("{}", &output[0..len - 1]); + } + + Ok(()) } else { if matches.is_present(OPT_SHORT) || matches.is_present(OPT_DOMAIN) { let mut it = hostname.char_indices().filter(|&ci| ci.1 == '.');