wall: testing

- added some argument tests.
- used `cfg` directives to build (no-op) on windows.
- minor readability tweaks

Author: 0jdxt

src/uu/wall/src/wall.rs

@@ -1,3 +1,4 @@
+#![cfg(unix)]
 #![warn(clippy::all, clippy::pedantic)]
 
 use std::{
@@ -76,7 +77,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
         return Err(USimpleError::new(1, "invalid timeout argument: 0"));
     }
 
-    // get nobanner flag and check user is root
+    // get nobanner flag and check if user is root
     let flag = args.get_flag("nobanner");
     let print_banner = if flag && unsafe { libc::geteuid() } != 0 {
         eprintln!("wall: --nobanner is available only for root");
@@ -94,41 +95,40 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
     // If we have a single input arg and it exists on disk, treat as a file.
     // If either is false, assume it is a literal string.
     // If no input given, use stdin.
-    match args.get_many::<String>("input") {
-        Some(v) => {
-            let vals: Vec<&str> = v.map(String::as_str).collect();
-
-            let fname = vals
-                .first()
-                .expect("clap guarantees at least 1 value for input");
-
-            let p = Path::new(fname);
-            if vals.len() == 1 && p.exists() {
-                // When we are not root, but suid or sgid, refuse to read files
-                // (e.g. device files) that the user may not have access to.
-                // After all, our invoker can easily do "wall < file"
-                // instead of "wall file".
-                unsafe {
-                    let uid = libc::getuid();
-                    if uid > 0 && (uid != libc::geteuid() || libc::getgid() != libc::getegid()) {
-                        return Err(USimpleError::new(
-                            1,
-                            format!("will not read {fname} - use stdin"),
-                        ));
-                    }
+    if let Some(v) = args.get_many::<String>("input") {
+        let vals: Vec<&str> = v.map(String::as_str).collect();
+
+        let fname = vals
+            .first()
+            .expect("clap guarantees at least 1 value for input");
+
+        let p = Path::new(fname);
+        if vals.len() == 1 && p.exists() {
+            // When we are not root, but suid or sgid, refuse to read files
+            // (e.g. device files) that the user may not have access to.
+            // After all, our invoker can easily do "wall < file" instead of "wall file".
+            unsafe {
+                let uid = libc::getuid();
+                if uid > 0 && (uid != libc::geteuid() || libc::getgid() != libc::getegid()) {
+                    return Err(USimpleError::new(
+                        1,
+                        format!("will not read {fname} - use stdin"),
+                    ));
                 }
+            }
 
-                let f = File::open(p)
-                    .map_err(|_| USimpleError::new(1, format!("cannot open {fname}")))?;
+            let Ok(f) = File::open(p) else {
+                return Err(USimpleError::new(1, format!("cannot open {fname}")));
+            };
 
-                wall(f, group, timeout, print_banner);
-            } else {
-                let mut s = vals.as_slice().join(" ");
-                s.push('\n');
-                wall(s.as_bytes(), group, timeout, print_banner);
-            }
+            wall(f, group, timeout, print_banner);
+        } else {
+            let mut s = vals.as_slice().join(" ");
+            s.push('\n');
+            wall(s.as_bytes(), group, timeout, print_banner);
         }
-        None => wall(stdin(), group, timeout, print_banner),
+    } else {
+        wall(stdin(), group, timeout, print_banner);
     }
 
     Ok(())
@@ -294,9 +294,8 @@ fn sanitise_line(line: &str) -> String {
 }
 
 // Determine if user is in specified group
+#[allow(clippy::cast_sign_loss)]
 fn is_gr_member(user: &[c_char], gid: gid_t) -> bool {
-    #![allow(clippy::cast_sign_loss)]
-
     // make sure user exists in database
     let pw = unsafe { libc::getpwnam(user.as_ptr()) };
     if pw.is_null() {
@@ -309,12 +308,14 @@ fn is_gr_member(user: &[c_char], gid: gid_t) -> bool {
         return true;
     }
 
+    // on macos, getgrouplist takes c_int as its group argument,
+    // other unices should use gid_t like linux
     #[cfg(target_os = "macos")]
     let base_gid = group as libc::c_int;
     #[cfg(not(target_os = "macos"))]
     let base_gid = group;
 
-    // otherwise check gid is in list of groups user belongs to
+    // otherwise check gid is in list of supplementary groups user belongs to
     let mut ngroups = 16;
     let mut groups: Vec<gid_t> = vec![0; ngroups as usize];
     while unsafe {
@@ -336,19 +337,19 @@ fn is_gr_member(user: &[c_char], gid: gid_t) -> bool {
 // Try to get corresponding group gid.
 fn get_group_gid(group: &String) -> UResult<gid_t> {
     // first we try as a group name
-    let cname =
-        CString::from_str(group).map_err(|_| USimpleError::new(1, "invalid group argument"))?;
+    let Ok(cname) = CString::from_str(group) else {
+        return Err(USimpleError::new(1, "invalid group argument"));
+    };
 
     let gr = unsafe { libc::getgrnam(cname.as_ptr()) };
     if !gr.is_null() {
         return Ok(unsafe { (*gr).gr_gid });
     }
 
     // otherwise, try as literal gid
-    let gid = group
-        .parse::<gid_t>()
-        .map_err(|_| USimpleError::new(1, "invalid group argument"))?;
-
+    let Ok(gid) = group.parse::<gid_t>() else {
+        return Err(USimpleError::new(1, "invalid group argument"));
+    };
     if unsafe { libc::getgrgid(gid) }.is_null() {
         return Err(USimpleError::new(1, format!("{group}: unknown gid")));
     }

tests/by-util/test_wall.rs

@@ -0,0 +1,50 @@
+#[cfg(unix)]
+mod tests {
+    use uutests::new_ucmd;
+
+    #[test]
+    fn test_invalid_arg() { new_ucmd!().arg("--definitely-invalid").fails().code_is(1); }
+
+    #[test]
+    fn test_fails_on_invalid_group() {
+        new_ucmd!()
+            .arg("-g")
+            .arg("fooblywoobly") // assuming this group doesnt exist
+            .fails()
+            .code_is(1)
+            .stderr_contains("wall: invalid group argument");
+    }
+
+    #[test]
+    fn test_fails_on_invalid_gid() {
+        new_ucmd!()
+            .arg("-g")
+            .arg("99999") // assuming this group doesnt exist
+            .fails()
+            .code_is(1)
+            .stderr_contains("wall: 99999: unknown gid");
+    }
+
+    #[test]
+    fn test_warns_on_nobanner() {
+        new_ucmd!()
+            .arg("-n")
+            .arg("some text to wall")
+            .succeeds()
+            .code_is(0)
+            .stderr_contains("wall: --nobanner is available only for root");
+    }
+
+    #[test]
+    fn test_fails_on_invalid_timeout() {
+        new_ucmd!()
+            .arg("-t")
+            .arg("0")
+            .fails()
+            .code_is(1)
+            .stderr_contains("wall: invalid timeout argument: 0");
+    }
+
+    #[test]
+    fn test_succeeds_no_stdout() { new_ucmd!().pipe_in("pipe me").succeeds().stdout_is(""); }
+}

tests/tests.rs

@@ -90,3 +90,7 @@ mod test_mcookie;
 #[cfg(feature = "uuidgen")]
 #[path = "by-util/test_uuidgen.rs"]
 mod test_uuidgen;
+
+#[cfg(feature = "wall")]
+#[path = "by-util/test_wall.rs"]
+mod test_wall;