diff --git a/.github/workflows/CICD.yml b/.github/workflows/CICD.yml index 93f1fac7992..8fde2b36152 100644 --- a/.github/workflows/CICD.yml +++ b/.github/workflows/CICD.yml @@ -746,9 +746,11 @@ jobs: # The account also has empty gecos fields. # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. - FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7") | sudo utmpdump -r -o /var/run/utmp + printf '%s\n%s\n%s' \ + '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ + '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for @@ -1107,9 +1109,11 @@ jobs: # The account also has empty gecos fields. # To work around these issues for pinky (and who) tests, we create a fake utmp file with a # system boot entry and a login entry for the GH runner account. - FAKE_UTMP_2='[2] [00000] [~~ ] [reboot] [~ ] [6.0.0-test] [0.0.0.0] [2022-02-22T22:11:22,222222+00:00]' - FAKE_UTMP_7='[7] [999999] [tty2] [runner] [tty2] [ ] [0.0.0.0] [2022-02-22T22:22:22,222222+00:00]' - (echo "$FAKE_UTMP_2" ; echo "$FAKE_UTMP_7") | sudo utmpdump -r -o /var/run/utmp + printf '%s\n%s\n%s' \ + '[2] [00000] [~~ ] [reboot ] [~ ] [6.0.0-test ] [0.0.0.0 ] [2022-02-22T22:11:22,222222+0000]' \ + '[7] [999999] [tty2] [runner ] [tty2 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + '[7] [00001] [tty3] [runner ] [tty3 ] [ ] [0.0.0.0 ] [2022-02-22T22:22:22,222222+0000]' \ + | sudo utmpdump -r -o /var/run/utmp # ... and add a full name to each account with a gecos field but no full name. sudo sed -i 's/:,/:runner name,/' /etc/passwd # We also create a couple optional files pinky looks for diff --git a/src/uu/users/Cargo.toml b/src/uu/users/Cargo.toml index fa311e74ac4..c86f0146c13 100644 --- a/src/uu/users/Cargo.toml +++ b/src/uu/users/Cargo.toml @@ -24,7 +24,7 @@ path = "src/users.rs" [dependencies] clap = { workspace = true } -uucore = { workspace = true, features = ["utmpx"] } +uucore = { workspace = true, features = ["utmpx", "process"] } fluent = { workspace = true } [target.'cfg(target_os = "openbsd")'.dependencies] diff --git a/src/uu/users/src/users.rs b/src/uu/users/src/users.rs index 3fb48a9b613..79343d254ed 100644 --- a/src/uu/users/src/users.rs +++ b/src/uu/users/src/users.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (paths) wtmp +// spell-checker:ignore (paths) wtmp ESRCH use std::ffi::OsString; use std::path::Path; @@ -12,6 +12,8 @@ use clap::builder::ValueParser; use clap::{Arg, Command}; use uucore::error::UResult; use uucore::format_usage; +#[cfg(not(target_os = "openbsd"))] +use uucore::process::pid_is_alive; use uucore::translate; #[cfg(target_os = "openbsd")] @@ -66,7 +68,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let filename = maybe_file.unwrap_or(utmpx::DEFAULT_FILE.as_ref()); users = Utmpx::iter_all_records_from(filename) - .filter(|ut| ut.is_user_process()) + .filter(|ut| ut.is_user_process() && pid_is_alive(ut.pid())) .map(|ut| ut.user()) .collect::>(); }; diff --git a/src/uu/who/Cargo.toml b/src/uu/who/Cargo.toml index fdb5dd41dfc..8a8ac22bde7 100644 --- a/src/uu/who/Cargo.toml +++ b/src/uu/who/Cargo.toml @@ -25,7 +25,7 @@ path = "src/who.rs" [dependencies] clap = { workspace = true } -uucore = { workspace = true, features = ["utmpx"] } +uucore = { workspace = true, features = ["utmpx", "process"] } fluent = { workspace = true } [[bin]] diff --git a/src/uu/who/locales/en-US.ftl b/src/uu/who/locales/en-US.ftl index f092489a16a..aa939de3ebe 100644 --- a/src/uu/who/locales/en-US.ftl +++ b/src/uu/who/locales/en-US.ftl @@ -26,10 +26,7 @@ who-help-users = list users logged in who-help-mesg = add user's message status as +, - or ? # Output messages -who-user-count = # { $count -> - [one] user={ $count } - *[other] users={ $count } -} +who-user-count = # users={ $count } # Idle time indicators who-idle-current = . diff --git a/src/uu/who/locales/fr-FR.ftl b/src/uu/who/locales/fr-FR.ftl index ca88a4946c1..92b70806995 100644 --- a/src/uu/who/locales/fr-FR.ftl +++ b/src/uu/who/locales/fr-FR.ftl @@ -25,10 +25,7 @@ who-help-users = liste les utilisateurs connectés who-help-mesg = ajoute le statut de message de l'utilisateur comme +, - ou ? # Output messages -who-user-count = # { $count -> - [one] utilisateur={ $count } - *[other] utilisateurs={ $count } -} +who-user-count = # utilisateurs={ $count } # Idle time indicators who-idle-old = anc. diff --git a/src/uu/who/src/platform/unix.rs b/src/uu/who/src/platform/unix.rs index 8e72a83ba39..1df818a91dd 100644 --- a/src/uu/who/src/platform/unix.rs +++ b/src/uu/who/src/platform/unix.rs @@ -3,7 +3,7 @@ // For the full copyright and license information, please view the LICENSE // file that was distributed with this source code. -// spell-checker:ignore (ToDO) ttyname hostnames runlevel mesg wtmp statted boottime deadprocs initspawn clockchange curr pidstr exitstr hoststr +// spell-checker:ignore (ToDO) ttyname hostnames runlevel mesg wtmp statted boottime deadprocs initspawn clockchange curr pidstr exitstr hoststr ESRCH use crate::options; use crate::uu_app; @@ -11,6 +11,8 @@ use crate::uu_app; use uucore::display::Quotable; use uucore::error::{FromIo, UResult}; use uucore::libc::{S_IWGRP, STDIN_FILENO, ttyname}; +#[cfg(not(target_os = "openbsd"))] +use uucore::process::pid_is_alive; use uucore::translate; use uucore::utmpx::{self, UtmpxRecord, time}; @@ -210,7 +212,7 @@ impl Who { }; if self.short_list { let users = utmpx::Utmpx::iter_all_records_from(f) - .filter(|ut| ut.is_user_process()) + .filter(|ut| ut.is_user_process() && pid_is_alive(ut.pid())) .map(|ut| ut.user()) .collect::>(); println!("{}", users.join(" ")); @@ -229,7 +231,7 @@ impl Who { for ut in records { if !self.my_line_only || cur_tty == ut.tty_device() { - if self.need_users && ut.is_user_process() { + if self.need_users && ut.is_user_process() && pid_is_alive(ut.pid()) { self.print_user(&ut)?; } else { match ut.record_type() { diff --git a/src/uucore/src/lib/features/process.rs b/src/uucore/src/lib/features/process.rs index 55e8c36482b..1019dfc5ccf 100644 --- a/src/uucore/src/lib/features/process.rs +++ b/src/uucore/src/lib/features/process.rs @@ -51,6 +51,30 @@ pub fn getpid() -> pid_t { unsafe { libc::getpid() } } +/// Check if a process with the given PID is alive. +/// +/// Uses `kill(pid, 0)` which sends signal 0 (null signal) to check process existence +/// without actually sending a signal. This is a standard POSIX technique for checking +/// if a process exists. +/// +/// Returns `true` if: +/// - The process exists (kill returns 0) +/// - We lack permission to signal the process (errno != ESRCH) +/// This means the process exists but we can't signal it +/// +/// Returns `false` only if: +/// - errno is ESRCH (No such process), confirming the process doesn't exist +/// +/// PIDs <= 0 are considered alive for compatibility with utmp records that may +/// contain special or invalid PID values. +#[cfg(not(target_os = "openbsd"))] +pub fn pid_is_alive(pid: i32) -> bool { + if pid <= 0 { + return true; + } + unsafe { libc::kill(pid, 0) == 0 || Errno::last() != Errno::ESRCH } +} + /// `getsid()` returns the session ID of the process with process ID pid. /// /// If pid is 0, getsid() returns the session ID of the calling process. diff --git a/tests/by-util/test_users.rs b/tests/by-util/test_users.rs index 0d3d7772b83..ab586029f8a 100644 --- a/tests/by-util/test_users.rs +++ b/tests/by-util/test_users.rs @@ -4,7 +4,7 @@ // file that was distributed with this source code. use uutests::new_ucmd; #[cfg(any(target_vendor = "apple", target_os = "linux"))] -use uutests::{util::TestScenario, util_name}; +use uutests::{unwrap_or_return, util::TestScenario, util::expected_result, util_name}; #[test] fn test_invalid_arg() { @@ -18,20 +18,10 @@ fn test_users_no_arg() { #[test] #[cfg(any(target_vendor = "apple", target_os = "linux"))] -#[ignore = "issue #3219"] fn test_users_check_name() { - #[cfg(target_os = "linux")] - let util_name = util_name!(); - #[cfg(target_vendor = "apple")] - let util_name = &format!("g{}", util_name!()); - - let expected = TestScenario::new(util_name) - .cmd(util_name) - .env("LC_ALL", "C") - .succeeds() - .stdout_move_str(); - - new_ucmd!().succeeds().stdout_is(&expected); + let ts = TestScenario::new(util_name!()); + let expected_stdout = unwrap_or_return!(expected_result(&ts, &[])).stdout_move_str(); + ts.ucmd().succeeds().stdout_is(expected_stdout); } #[test] diff --git a/tests/by-util/test_who.rs b/tests/by-util/test_who.rs index d94a7b99a8f..4fd9a5996d3 100644 --- a/tests/by-util/test_who.rs +++ b/tests/by-util/test_who.rs @@ -16,7 +16,6 @@ fn test_invalid_arg() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_count() { let ts = TestScenario::new(util_name!()); for opt in ["-q", "--count", "--c"] { @@ -42,7 +41,6 @@ fn test_boot() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_heading() { let ts = TestScenario::new(util_name!()); for opt in ["-H", "--heading", "--head"] { @@ -61,7 +59,6 @@ fn test_heading() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_short() { let ts = TestScenario::new(util_name!()); for opt in ["-s", "--short", "--s"] { @@ -128,7 +125,6 @@ fn test_time() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_mesg() { // -T, -w, --mesg // add user's message status as +, - or ? @@ -172,7 +168,6 @@ fn test_too_many_args() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_users() { let ts = TestScenario::new(util_name!()); for opt in ["-u", "--users", "--us"] { @@ -198,7 +193,6 @@ fn test_users() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_lookup() { let opt = "--lookup"; let ts = TestScenario::new(util_name!()); @@ -219,7 +213,6 @@ fn test_dead() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_all_separately() { if cfg!(target_os = "macos") { // TODO: fix `-u`, see: test_users @@ -237,7 +230,6 @@ fn test_all_separately() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] fn test_all() { if cfg!(target_os = "macos") { // TODO: fix `-u`, see: test_users @@ -253,7 +245,7 @@ fn test_all() { #[cfg(unix)] #[test] -#[ignore = "issue #3219"] +#[ignore = "This test is the only one in the suite that checks if the reference version is 8.3. This has not been the case for a while"] fn test_locale() { let ts = TestScenario::new(util_name!());