From 4555c8556405d18c15385c4d5fbe5dbd328f4ee4 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Fri, 27 Aug 2021 20:36:26 +0200 Subject: [PATCH 1/3] whoami: Cleanup - Use modern conventions - Restrict the scope of unsafe - Do not use deprecated `std::mem::unitialized()` - Do not bake unicode into design --- Cargo.lock | 1 + src/uu/whoami/Cargo.toml | 5 ++- src/uu/whoami/src/platform/unix.rs | 14 ++++---- src/uu/whoami/src/platform/windows.rs | 31 +++++++++-------- src/uu/whoami/src/whoami.rs | 49 +++++++-------------------- 5 files changed, 41 insertions(+), 59 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 91e2f1762..d767526ab 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3222,6 +3222,7 @@ name = "uu_whoami" version = "0.0.7" dependencies = [ "clap", + "libc", "uucore", "uucore_procs", "winapi 0.3.9", diff --git a/src/uu/whoami/Cargo.toml b/src/uu/whoami/Cargo.toml index 919aab2e5..d12ea1aea 100644 --- a/src/uu/whoami/Cargo.toml +++ b/src/uu/whoami/Cargo.toml @@ -16,12 +16,15 @@ path = "src/whoami.rs" [dependencies] clap = { version = "2.33", features = ["wrap_help"] } -uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries", "wide"] } +uucore = { version=">=0.0.9", package="uucore", path="../../uucore", features=["entries"] } uucore_procs = { version=">=0.0.6", package="uucore_procs", path="../../uucore_procs" } [target.'cfg(target_os = "windows")'.dependencies] winapi = { version = "0.3", features = ["lmcons"] } +[target.'cfg(unix)'.dependencies] +libc = "0.2.42" + [[bin]] name = "whoami" path = "src/main.rs" diff --git a/src/uu/whoami/src/platform/unix.rs b/src/uu/whoami/src/platform/unix.rs index 3d5fc6f54..1c0ea15d5 100644 --- a/src/uu/whoami/src/platform/unix.rs +++ b/src/uu/whoami/src/platform/unix.rs @@ -8,14 +8,14 @@ * file that was distributed with this source code. */ -// spell-checker:ignore (ToDO) getusername +use std::ffi::OsString; +use std::io; -use std::io::Result; use uucore::entries::uid2usr; -use uucore::libc::geteuid; -pub unsafe fn get_username() -> Result { - // Get effective user id - let uid = geteuid(); - uid2usr(uid) +pub fn get_username() -> io::Result { + // SAFETY: getuid() does nothing with memory and is always successful. + let uid = unsafe { libc::geteuid() }; + // uid2usr should arguably return an OsString but currently doesn't + uid2usr(uid).map(Into::into) } diff --git a/src/uu/whoami/src/platform/windows.rs b/src/uu/whoami/src/platform/windows.rs index 3fe8eb1e7..8afa18b63 100644 --- a/src/uu/whoami/src/platform/windows.rs +++ b/src/uu/whoami/src/platform/windows.rs @@ -7,22 +7,23 @@ * file that was distributed with this source code. */ -extern crate winapi; +use std::ffi::OsString; +use std::io; +use std::os::windows::ffi::OsStringExt; -use self::winapi::shared::lmcons; -use self::winapi::shared::minwindef; -use self::winapi::um::{winbase, winnt}; -use std::io::{Error, Result}; -use std::mem; -use uucore::wide::FromWide; +use winapi::shared::lmcons; +use winapi::shared::minwindef::DWORD; +use winapi::um::winbase; -pub unsafe fn get_username() -> Result { - #[allow(deprecated)] - let mut buffer: [winnt::WCHAR; lmcons::UNLEN as usize + 1] = mem::uninitialized(); - let mut len = buffer.len() as minwindef::DWORD; - if winbase::GetUserNameW(buffer.as_mut_ptr(), &mut len) == 0 { - return Err(Error::last_os_error()); +pub fn get_username() -> io::Result { + const BUF_LEN: DWORD = lmcons::UNLEN + 1; + let mut buffer = [0_u16; BUF_LEN as usize]; + let mut len = BUF_LEN; + // SAFETY: buffer.len() == len + unsafe { + if winbase::GetUserNameW(buffer.as_mut_ptr(), &mut len) == 0 { + return Err(io::Error::last_os_error()); + } } - let username = String::from_wide(&buffer[..len as usize - 1]); - Ok(username) + Ok(OsString::from_wide(&buffer[..len as usize - 1])) } diff --git a/src/uu/whoami/src/whoami.rs b/src/uu/whoami/src/whoami.rs index 3033a078a..830b86e63 100644 --- a/src/uu/whoami/src/whoami.rs +++ b/src/uu/whoami/src/whoami.rs @@ -1,5 +1,3 @@ -use clap::App; - // * This file is part of the uutils coreutils package. // * // * (c) Jordi Boggiano @@ -14,46 +12,25 @@ extern crate clap; #[macro_use] extern crate uucore; -use uucore::error::{UResult, USimpleError}; +use clap::App; + +use uucore::display::println_verbatim; +use uucore::error::{FromIo, UResult}; mod platform; +static ABOUT: &str = "Print the current username."; + #[uucore_procs::gen_uumain] pub fn uumain(args: impl uucore::Args) -> UResult<()> { - let app = uu_app(); - - if let Err(err) = app.get_matches_from_safe(args) { - if err.kind == clap::ErrorKind::HelpDisplayed - || err.kind == clap::ErrorKind::VersionDisplayed - { - println!("{}", err); - Ok(()) - } else { - return Err(USimpleError::new(1, format!("{}", err))); - } - } else { - exec() - } + uu_app().get_matches_from(args); + let username = platform::get_username().map_err_context(|| "failed to get username".into())?; + println_verbatim(&username).map_err_context(|| "failed to print username".into())?; + Ok(()) } pub fn uu_app() -> App<'static, 'static> { - app_from_crate!() -} - -pub fn exec() -> UResult<()> { - unsafe { - match platform::get_username() { - Ok(username) => { - println!("{}", username); - Ok(()) - } - Err(err) => match err.raw_os_error() { - Some(0) | None => Err(USimpleError::new(1, "failed to get username")), - Some(_) => Err(USimpleError::new( - 1, - format!("failed to get username: {}", err), - )), - }, - } - } + App::new(uucore::util_name()) + .version(crate_version!()) + .about(ABOUT) } From 0a3785bf8411be00e1032b51eb9b599852ccb979 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Tue, 14 Sep 2021 12:25:17 +0200 Subject: [PATCH 2/3] whoami: Run tests on Windows --- tests/by-util/test_whoami.rs | 2 -- tests/common/util.rs | 10 ++++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/tests/by-util/test_whoami.rs b/tests/by-util/test_whoami.rs index 3e8d5afa6..340c1434f 100644 --- a/tests/by-util/test_whoami.rs +++ b/tests/by-util/test_whoami.rs @@ -3,7 +3,6 @@ // * For the full copyright and license information, please view the LICENSE // * file that was distributed with this source code. -#[cfg(unix)] use crate::common::util::*; #[test] @@ -34,7 +33,6 @@ fn test_normal_compare_id() { } #[test] -#[cfg(unix)] fn test_normal_compare_env() { let whoami = whoami(); if whoami == "nobody" { diff --git a/tests/common/util.rs b/tests/common/util.rs index 704e9b163..f3cdec010 100644 --- a/tests/common/util.rs +++ b/tests/common/util.rs @@ -1069,10 +1069,12 @@ pub fn whoami() -> String { // Use environment variable to get current user instead of // invoking `whoami` and fall back to user "nobody" on error. - std::env::var("USER").unwrap_or_else(|e| { - println!("{}: {}, using \"nobody\" instead", UUTILS_WARNING, e); - "nobody".to_string() - }) + std::env::var("USER") + .or_else(|_| std::env::var("USERNAME")) + .unwrap_or_else(|e| { + println!("{}: {}, using \"nobody\" instead", UUTILS_WARNING, e); + "nobody".to_string() + }) } /// Add prefix 'g' for `util_name` if not on linux From 5bb56ec528f4ed29a021777a4a36ecd4e765fd27 Mon Sep 17 00:00:00 2001 From: Jan Verbeek Date: Wed, 15 Sep 2021 15:37:15 +0200 Subject: [PATCH 3/3] whoami: Restrict scope of unsafe Co-authored-by: Jan Scheer --- src/uu/whoami/src/platform/windows.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/uu/whoami/src/platform/windows.rs b/src/uu/whoami/src/platform/windows.rs index 8afa18b63..a627bed8e 100644 --- a/src/uu/whoami/src/platform/windows.rs +++ b/src/uu/whoami/src/platform/windows.rs @@ -20,10 +20,8 @@ pub fn get_username() -> io::Result { let mut buffer = [0_u16; BUF_LEN as usize]; let mut len = BUF_LEN; // SAFETY: buffer.len() == len - unsafe { - if winbase::GetUserNameW(buffer.as_mut_ptr(), &mut len) == 0 { - return Err(io::Error::last_os_error()); - } + if unsafe { winbase::GetUserNameW(buffer.as_mut_ptr(), &mut len) } == 0 { + return Err(io::Error::last_os_error()); } Ok(OsString::from_wide(&buffer[..len as usize - 1])) }