From fffff64cf51090eb0e49cc34d1c74317dab9c234 Mon Sep 17 00:00:00 2001 From: Tuomas Tynkkynen Date: Sat, 17 May 2025 01:55:19 +0300 Subject: [PATCH] pgrep/pkill/pidwait: Implement --pidfile --- src/uu/pgrep/src/pgrep.rs | 2 +- src/uu/pgrep/src/process_matcher.rs | 58 +++++++++++++++++++++++---- src/uu/pidwait/src/pidwait.rs | 2 +- src/uu/pkill/src/pkill.rs | 2 +- tests/by-util/test_pgrep.rs | 62 +++++++++++++++++++++++++++++ 5 files changed, 115 insertions(+), 11 deletions(-) diff --git a/src/uu/pgrep/src/pgrep.rs b/src/uu/pgrep/src/pgrep.rs index 116674a..f0d31a1 100644 --- a/src/uu/pgrep/src/pgrep.rs +++ b/src/uu/pgrep/src/pgrep.rs @@ -32,7 +32,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { settings.threads = matches.get_flag("lightweight"); // Collect pids - let pids = process_matcher::find_matching_pids(&settings); + let pids = process_matcher::find_matching_pids(&settings)?; // Processing output let output = if matches.get_flag("count") { diff --git a/src/uu/pgrep/src/process_matcher.rs b/src/uu/pgrep/src/process_matcher.rs index 49815a2..c362a7e 100644 --- a/src/uu/pgrep/src/process_matcher.rs +++ b/src/uu/pgrep/src/process_matcher.rs @@ -5,6 +5,7 @@ // Common process matcher logic shared by pgrep, pkill and pidwait +use std::fs; use std::hash::Hash; use std::{collections::HashSet, io}; @@ -46,6 +47,8 @@ pub struct Settings { pub session: Option>, pub cgroup: Option>, pub threads: bool, + + pub pidfile: Option, } pub fn get_match_settings(matches: &ArgMatches) -> UResult { @@ -106,6 +109,7 @@ pub fn get_match_settings(matches: &ArgMatches) -> UResult { .get_many::("cgroup") .map(|groups| groups.cloned().collect()), threads: false, + pidfile: matches.get_one::("pidfile").cloned(), }; if !settings.newest @@ -121,6 +125,7 @@ pub fn get_match_settings(matches: &ArgMatches) -> UResult { && settings.session.is_none() && settings.cgroup.is_none() && !settings.require_handler + && settings.pidfile.is_none() && pattern.is_empty() { return Err(USimpleError::new( @@ -142,13 +147,13 @@ pub fn get_match_settings(matches: &ArgMatches) -> UResult { Ok(settings) } -pub fn find_matching_pids(settings: &Settings) -> Vec { - let mut pids = collect_matched_pids(settings); +pub fn find_matching_pids(settings: &Settings) -> UResult> { + let mut pids = collect_matched_pids(settings)?; if pids.is_empty() { uucore::error::set_exit_code(1); - pids + Ok(pids) } else { - process_flag_o_n(settings, &mut pids) + Ok(process_flag_o_n(settings, &mut pids)) } } @@ -189,7 +194,7 @@ fn any_matches(optional_ids: &Option>, id: T) -> bool { } /// Collect pids with filter construct from command line arguments -fn collect_matched_pids(settings: &Settings) -> Vec { +fn collect_matched_pids(settings: &Settings) -> UResult> { // Filtration general parameters let filtered: Vec = { let mut tmp_vec = Vec::new(); @@ -200,6 +205,11 @@ fn collect_matched_pids(settings: &Settings) -> Vec { walk_process().collect::>() }; let our_pid = std::process::id() as usize; + let pid_from_pidfile = settings + .pidfile + .as_ref() + .map(|filename| read_pidfile(filename)) + .transpose()?; for mut pid in pids { if pid.pid == our_pid { @@ -267,6 +277,8 @@ fn collect_matched_pids(settings: &Settings) -> Vec { #[cfg(not(unix))] let handler_matched = true; + let pidfile_matched = pid_from_pidfile.is_none_or(|p| p == pid.pid as i64); + if (run_state_matched && pattern_matched && tty_matched @@ -276,7 +288,8 @@ fn collect_matched_pids(settings: &Settings) -> Vec { && session_matched && cgroup_matched && ids_matched - && handler_matched) + && handler_matched + && pidfile_matched) ^ settings.inverse { tmp_vec.push(pid); @@ -285,7 +298,7 @@ fn collect_matched_pids(settings: &Settings) -> Vec { tmp_vec }; - filtered + Ok(filtered) } /// Sorting pids for flag `-o` and `-n`. @@ -379,6 +392,35 @@ fn parse_gid_or_group_name(gid_or_group_name: &str) -> io::Result { .map_err(|_| io::Error::new(io::ErrorKind::InvalidInput, "invalid group name")) } +pub fn parse_pidfile_content(content: &str) -> Option { + let re = Regex::new(r"(?-m)^[[:blank:]]*(-?[0-9]+)(?:\s|$)").unwrap(); + re.captures(content)?.get(1)?.as_str().parse::().ok() +} + +#[test] +fn test_parse_pidfile_content_valid() { + assert_eq!(parse_pidfile_content(" 1234"), Some(1234)); + assert_eq!(parse_pidfile_content("-5678 "), Some(-5678)); + assert_eq!(parse_pidfile_content(" 42\nfoo\n"), Some(42)); + assert_eq!(parse_pidfile_content("\t-99\tbar\n"), Some(-99)); + + assert_eq!(parse_pidfile_content(""), None); + assert_eq!(parse_pidfile_content("abc"), None); + assert_eq!(parse_pidfile_content("0x42"), None); + assert_eq!(parse_pidfile_content("2.3"), None); + assert_eq!(parse_pidfile_content("\n123\n"), None); +} + +pub fn read_pidfile(filename: &str) -> UResult { + let content = fs::read_to_string(filename) + .map_err(|e| USimpleError::new(1, format!("Failed to read pidfile {}: {}", filename, e)))?; + + let pid = parse_pidfile_content(&content) + .ok_or_else(|| USimpleError::new(1, format!("Pidfile {} not valid", filename)))?; + + Ok(pid) +} + #[allow(clippy::cognitive_complexity)] pub fn clap_args(pattern_help: &'static str, enable_v_flag: bool) -> Vec { vec![ @@ -419,7 +461,7 @@ pub fn clap_args(pattern_help: &'static str, enable_v_flag: bool) -> Vec { .value_delimiter(',') .value_parser(parse_uid_or_username), arg!(-x --exact "match exactly with the command name"), - // arg!(-F --pidfile "read PIDs from file"), + arg!(-F --pidfile "read PIDs from file"), // arg!(-L --logpidfile "fail if PID file is not locked"), arg!(-r --runstates "match runstates [D,S,Z,...]"), // arg!(-A --"ignore-ancestors" "exclude our ancestors from results"), diff --git a/src/uu/pidwait/src/pidwait.rs b/src/uu/pidwait/src/pidwait.rs index 1a110a6..a3f4d37 100644 --- a/src/uu/pidwait/src/pidwait.rs +++ b/src/uu/pidwait/src/pidwait.rs @@ -18,7 +18,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { let matches = uu_app().try_get_matches_from(args)?; let settings = process_matcher::get_match_settings(&matches)?; - let mut proc_infos = process_matcher::find_matching_pids(&settings); + let mut proc_infos = process_matcher::find_matching_pids(&settings)?; // For empty result if proc_infos.is_empty() { diff --git a/src/uu/pkill/src/pkill.rs b/src/uu/pkill/src/pkill.rs index 07b8b0e..1405f3f 100644 --- a/src/uu/pkill/src/pkill.rs +++ b/src/uu/pkill/src/pkill.rs @@ -53,7 +53,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> { }; // Collect pids - let pids = process_matcher::find_matching_pids(&settings); + let pids = process_matcher::find_matching_pids(&settings)?; // Send signal // TODO: Implement -q diff --git a/tests/by-util/test_pgrep.rs b/tests/by-util/test_pgrep.rs index f0247b6..5f168d7 100644 --- a/tests/by-util/test_pgrep.rs +++ b/tests/by-util/test_pgrep.rs @@ -541,3 +541,65 @@ fn test_threads() { .join() .unwrap(); } + +#[test] +#[cfg(target_os = "linux")] +fn test_pidfile_match() { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), "1\tfoo\n").unwrap(); + + new_ucmd!() + .arg("--pidfile") + .arg(temp_file.path()) + .succeeds() + .stdout_is("1\n"); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_pidfile_no_match() { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), " -1").unwrap(); + + new_ucmd!() + .arg("--pidfile") + .arg(temp_file.path()) + .fails() + .no_output(); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_pidfile_invert() { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), " -1").unwrap(); + + new_ucmd!() + .arg("-v") + .arg("--pidfile") + .arg(temp_file.path()) + .succeeds(); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_pidfile_invalid_content() { + let temp_file = tempfile::NamedTempFile::new().unwrap(); + std::fs::write(temp_file.path(), "0x1").unwrap(); + + new_ucmd!() + .arg("--pidfile") + .arg(temp_file.path()) + .fails() + .stderr_matches(&Regex::new("Pidfile .* not valid").unwrap()); +} + +#[test] +#[cfg(target_os = "linux")] +fn test_pidfile_nonexistent_file() { + new_ucmd!() + .arg("--pidfile") + .arg("/nonexistent/file") + .fails() + .stderr_contains("Failed to read pidfile"); +}