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()

.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

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 clap::{crate_version, App, Arg, ArgMatches};
-use winapi::shared::minwindef::MAKEWORD;
+
-#[cfg(windows)]
+use uucore::error::{FromIo, UResult};
-use winapi::um::winsock2::{WSACleanup, WSAStartup};
 
 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)]
+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<WsaHandle> {
+        let err = unsafe {
+            let mut data = std::mem::MaybeUninit::<WSADATA>::uninit();
+            WSAStartup(MAKEWORD(2, 2), data.as_mut_ptr())
+        };
+        if err != 0 {
+            Err(io::Error::from_raw_os_error(err))
+        } else {
+            Ok(WsaHandle(()))
+        }
+    }
+
+    impl Drop for WsaHandle {
+        fn drop(&mut self) {
             unsafe {
-        #[allow(deprecated)]
+                // This possibly returns an error but we can't handle it
-        let mut data = std::mem::uninitialized();
+                let _err = WSACleanup();
-        if WSAStartup(MAKEWORD(2, 2), &mut data as *mut _) != 0 {
-            return Err(UUsageError::new(
-                1,
-                "Failed to start Winsock 2.2".to_string(),
-            ));
             }
         }
-    let result = execute(args);
-    #[cfg(windows)]
-    unsafe {
-        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) => {
+        Some(host) => hostname::set(host).map_err_context(|| "failed to set hostname".to_owned()),
-            if let Err(err) = hostname::set(host) {
-                return Err(USimpleError::new(1, format!("{}", err)));
-            } else {
-                Ok(())
-            }
-        }
     }
 }
 
@@ -81,44 +83,53 @@ 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(
+        .arg(
-            "Display the short hostname (the portion before the first dot) if \
+            Arg::with_name(OPT_SHORT)
-             possible",
+                .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() {
+        let addresses = hostname
-            Ok(addresses) => {
+            .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 = format!("{}", addr);
+                let mut ip = addr.to_string();
                 if ip.ends_with(":1") {
                     let len = ip.len();
                     ip.truncate(len - 2);
@@ -134,11 +145,6 @@ fn display_hostname(matches: &ArgMatches) -> UResult<()> {
         }
 
         Ok(())
-            }
-            Err(f) => {
-                return Err(USimpleError::new(1, format!("{}", f)));
-            }
-        }
     } else {
         if matches.is_present(OPT_SHORT) || matches.is_present(OPT_DOMAIN) {
             let mut it = hostname.char_indices().filter(|&ci| ci.1 == '.');