Skip to content

Commit 7f610a2

Browse files
cakebakercakebaker
authored
Merge pull request #321 from dezgeg/pgrep_pkill_unification
Unify matching logic in pgrep and pkill
2 parents c28c2f3 + a1ef871 commit 7f610a2

File tree

4 files changed

+120
-60
lines changed

4 files changed

+120
-60
lines changed

‎src/uu/pgrep/src/pgrep.rs‎

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ use clap::{arg, crate_version, Arg, ArgAction, ArgGroup, ArgMatches, Command};
1010
use process::{walk_process, ProcessInformation, Teletype};
1111
use regex::Regex;
1212
use std::{collections::HashSet, sync::OnceLock};
13+
#[cfg(unix)]
14+
use uucore::{display::Quotable, signals::signal_by_name_or_value};
1315
use uucore::{
1416
error::{UResult, USimpleError},
1517
format_usage, help_about, help_usage,
@@ -87,9 +89,21 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
8789
));
8890
}
8991

92+
// Parse signal
93+
#[cfg(unix)]
94+
let sig_num = parse_signal_value(matches.get_one::<String>("signal").unwrap())?;
95+
9096
// Collect pids
9197
let pids = {
9298
let mut pids = collect_matched_pids(&settings);
99+
#[cfg(unix)]
100+
if matches.get_flag("require-handler") {
101+
pids.retain(|pid| {
102+
let mask =
103+
u64::from_str_radix(pid.clone().status().get("SigCgt").unwrap(), 16).unwrap();
104+
mask & (1 << sig_num) != 0
105+
});
106+
}
93107
if pids.is_empty() {
94108
uucore::error::set_exit_code(1);
95109
pids
@@ -170,7 +184,7 @@ fn collect_matched_pids(settings: &Settings) -> Vec<ProcessInformation> {
170184
let mut tmp_vec = Vec::new();
171185

172186
for mut pid in walk_process().collect::<Vec<_>>() {
173-
let run_state_matched = match (&settings.runstates, (pid).run_state()) {
187+
let run_state_matched = match (&settings.runstates, pid.run_state()) {
174188
(Some(arg_run_states), Ok(pid_state)) => {
175189
arg_run_states.contains(&pid_state.to_string())
176190
}
@@ -275,6 +289,12 @@ fn process_flag_o_n(
275289
}
276290
}
277291

292+
#[cfg(unix)]
293+
fn parse_signal_value(signal_name: &str) -> UResult<usize> {
294+
signal_by_name_or_value(signal_name)
295+
.ok_or_else(|| USimpleError::new(1, format!("Unknown signal {}", signal_name.quote())))
296+
}
297+
278298
#[allow(clippy::cognitive_complexity)]
279299
pub fn uu_app() -> Command {
280300
Command::new(uucore::util_name())
@@ -289,12 +309,17 @@ pub fn uu_app() -> Command {
289309
.hide_default_value(true),
290310
arg!(-l --"list-name" "list PID and process name"),
291311
arg!(-a --"list-full" "list PID and full command line"),
312+
arg!(-H --"require-handler" "match only if signal handler is present"),
292313
arg!(-v --inverse "negates the matching"),
293314
// arg!(-w --lightweight "list all TID"),
294315
arg!(-c --count "count of matching processes"),
295316
arg!(-f --full "use full process name to match"),
296-
// arg!(-g --pgroup <PGID> ... "match listed process group IDs"),
297-
// arg!(-G --group <GID> ... "match real group IDs"),
317+
// arg!(-g --pgroup <PGID> ... "match listed process group IDs")
318+
// .value_delimiter(',')
319+
// .value_parser(clap::value_parser!(u64)),
320+
// arg!(-G --group <GID> ... "match real group IDs")
321+
// .value_delimiter(',')
322+
// .value_parser(clap::value_parser!(u64)),
298323
arg!(-i --"ignore-case" "match case insensitively"),
299324
arg!(-n --newest "select most recently started"),
300325
arg!(-o --oldest "select least recently started"),
@@ -303,17 +328,30 @@ pub fn uu_app() -> Command {
303328
arg!(-P --parent <PPID> "match only child processes of the given parent")
304329
.value_delimiter(',')
305330
.value_parser(clap::value_parser!(u64)),
306-
// arg!(-s --session <SID> "match session IDs"),
331+
// arg!(-s --session <SID> "match session IDs")
332+
// .value_delimiter(',')
333+
// .value_parser(clap::value_parser!(u64)),
334+
arg!(--signal <sig> "signal to send (either number or name)")
335+
.default_value("SIGTERM"),
307336
arg!(-t --terminal <tty> "match by controlling terminal")
308337
.value_delimiter(','),
309-
// arg!(-u --euid <ID> ... "match by effective IDs"),
310-
// arg!(-U --uid <ID> ... "match by real IDs"),
338+
// arg!(-u --euid <ID> ... "match by effective IDs")
339+
// .value_delimiter(',')
340+
// .value_parser(clap::value_parser!(u64)),
341+
// arg!(-U --uid <ID> ... "match by real IDs")
342+
// .value_delimiter(',')
343+
// .value_parser(clap::value_parser!(u64)),
311344
arg!(-x --exact "match exactly with the command name"),
312345
// arg!(-F --pidfile <file> "read PIDs from file"),
313346
// arg!(-L --logpidfile "fail if PID file is not locked"),
314347
arg!(-r --runstates <state> "match runstates [D,S,Z,...]"),
348+
// arg!(-A --"ignore-ancestors" "exclude our ancestors from results"),
349+
// arg!(--cgroup <grp> "match by cgroup v2 names")
350+
// .value_delimiter(','),
315351
// arg!( --ns <PID> "match the processes that belong to the same namespace as <pid>"),
316-
// arg!( --nslist <ns> ... "list which namespaces will be considered for the --ns option."),
352+
// arg!( --nslist <ns> ... "list which namespaces will be considered for the --ns option.")
353+
// .value_delimiter(',')
354+
// .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]),
317355
])
318356
.arg(
319357
Arg::new("pattern")

‎src/uu/pkill/src/pkill.rs‎

Lines changed: 43 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct Settings {
3636
exact: bool,
3737
full: bool,
3838
ignore_case: bool,
39+
inverse: bool,
3940
newest: bool,
4041
oldest: bool,
4142
older: Option<u64>,
@@ -51,7 +52,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
5152
#[cfg(target_os = "windows")]
5253
let args = args.collect_ignore();
5354
#[cfg(unix)]
54-
let obs_signal = handle_obsolete(&mut args);
55+
handle_obsolete(&mut args);
5556

5657
let matches = uu_app().try_get_matches_from(&args)?;
5758

@@ -64,6 +65,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
6465
exact: matches.get_flag("exact"),
6566
full: matches.get_flag("full"),
6667
ignore_case: matches.get_flag("ignore-case"),
68+
inverse: matches.get_flag("inverse"),
6769
newest: matches.get_flag("newest"),
6870
oldest: matches.get_flag("oldest"),
6971
parent: matches
@@ -94,13 +96,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
9496

9597
// Parse signal
9698
#[cfg(unix)]
97-
let sig_num = if let Some(signal) = obs_signal {
98-
signal
99-
} else if let Some(signal) = matches.get_one::<String>("signal") {
100-
parse_signal_value(signal)?
101-
} else {
102-
15_usize //SIGTERM
103-
};
99+
let sig_num = parse_signal_value(matches.get_one::<String>("signal").unwrap())?;
104100

105101
#[cfg(unix)]
106102
let sig_name = signal_name_by_value(sig_num);
@@ -123,7 +119,7 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
123119
if matches.get_flag("require-handler") {
124120
pids.retain(|pid| {
125121
let mask =
126-
u32::from_str_radix(pid.clone().status().get("SigCgt").unwrap(), 16).unwrap();
122+
u64::from_str_radix(pid.clone().status().get("SigCgt").unwrap(), 16).unwrap();
127123
mask & (1 << sig_num) != 0
128124
});
129125
}
@@ -235,11 +231,12 @@ fn collect_matched_pids(settings: &Settings) -> Vec<ProcessInformation> {
235231
_ => true,
236232
};
237233

238-
if run_state_matched
234+
if (run_state_matched
239235
&& pattern_matched
240236
&& tty_matched
241237
&& older_matched
242-
&& parent_matched
238+
&& parent_matched)
239+
^ settings.inverse
243240
{
244241
tmp_vec.push(pid);
245242
}
@@ -290,7 +287,7 @@ fn process_flag_o_n(
290287
}
291288

292289
#[cfg(unix)]
293-
fn handle_obsolete(args: &mut Vec<String>) -> Option<usize> {
290+
fn handle_obsolete(args: &mut [String]) {
294291
// Sanity check
295292
if args.len() > 2 {
296293
// Old signal can only be in the first argument position
@@ -299,25 +296,17 @@ fn handle_obsolete(args: &mut Vec<String>) -> Option<usize> {
299296
// Check if it is a valid signal
300297
let opt_signal = signal_by_name_or_value(signal);
301298
if opt_signal.is_some() {
302-
// remove the signal before return
303-
args.remove(1);
304-
return opt_signal;
299+
// Replace with long option that clap can parse
300+
args[1] = format!("--signal={}", signal);
305301
}
306302
}
307303
}
308-
None
309304
}
310305

311306
#[cfg(unix)]
312307
fn parse_signal_value(signal_name: &str) -> UResult<usize> {
313-
let optional_signal_value = signal_by_name_or_value(signal_name);
314-
match optional_signal_value {
315-
Some(x) => Ok(x),
316-
None => Err(USimpleError::new(
317-
1,
318-
format!("Unknown signal {}", signal_name.quote()),
319-
)),
320-
}
308+
signal_by_name_or_value(signal_name)
309+
.ok_or_else(|| USimpleError::new(1, format!("Unknown signal {}", signal_name.quote())))
321310
}
322311

323312
#[cfg(unix)]
@@ -343,20 +332,21 @@ pub fn uu_app() -> Command {
343332
.about(ABOUT)
344333
.override_usage(format_usage(USAGE))
345334
.args_override_self(true)
346-
.group(ArgGroup::new("oldest_newest").args(["oldest", "newest"]))
335+
.group(ArgGroup::new("oldest_newest").args(["oldest", "newest", "inverse"]))
347336
.args([
348337
// arg!(-<sig> "signal to send (either number or name)"),
349338
arg!(-H --"require-handler" "match only if signal handler is present"),
350-
arg!(-q --queue <value> "integer value to be sent with the signal"),
339+
// arg!(-q --queue <value> "integer value to be sent with the signal"),
351340
arg!(-e --echo "display what is killed"),
341+
arg!(--inverse "negates the matching"),
352342
arg!(-c --count "count of matching processes"),
353343
arg!(-f --full "use full process name to match"),
354-
arg!(-g --pgroup <PGID> "match listed process group IDs")
355-
.value_delimiter(',')
356-
.value_parser(clap::value_parser!(u64)),
357-
arg!(-G --group <GID> "match real group IDs")
358-
.value_delimiter(',')
359-
.value_parser(clap::value_parser!(u64)),
344+
// arg!(-g --pgroup <PGID> "match listed process group IDs")
345+
// .value_delimiter(',')
346+
// .value_parser(clap::value_parser!(u64)),
347+
// arg!(-G --group <GID> "match real group IDs")
348+
// .value_delimiter(',')
349+
// .value_parser(clap::value_parser!(u64)),
360350
arg!(-i --"ignore-case" "match case insensitively"),
361351
arg!(-n --newest "select most recently started"),
362352
arg!(-o --oldest "select least recently started"),
@@ -365,29 +355,29 @@ pub fn uu_app() -> Command {
365355
arg!(-P --parent <PPID> "match only child processes of the given parent")
366356
.value_delimiter(',')
367357
.value_parser(clap::value_parser!(u64)),
368-
arg!(-s --session <SID> "match session IDs")
369-
.value_delimiter(',')
370-
.value_parser(clap::value_parser!(u64)),
371-
arg!(--signal <sig> "signal to send (either number or name)"),
372-
arg!(-t --terminal <tty> "match by controlling terminal")
373-
.value_delimiter(','),
374-
arg!(-u --euid <ID> "match by effective IDs")
375-
.value_delimiter(',')
376-
.value_parser(clap::value_parser!(u64)),
377-
arg!(-U --uid <ID> "match by real IDs")
378-
.value_delimiter(',')
379-
.value_parser(clap::value_parser!(u64)),
358+
// arg!(-s --session <SID> "match session IDs")
359+
// .value_delimiter(',')
360+
// .value_parser(clap::value_parser!(u64)),
361+
arg!(--signal <sig> "signal to send (either number or name)")
362+
.default_value("SIGTERM"),
363+
arg!(-t --terminal <tty> "match by controlling terminal").value_delimiter(','),
364+
// arg!(-u --euid <ID> "match by effective IDs")
365+
// .value_delimiter(',')
366+
// .value_parser(clap::value_parser!(u64)),
367+
// arg!(-U --uid <ID> "match by real IDs")
368+
// .value_delimiter(',')
369+
// .value_parser(clap::value_parser!(u64)),
380370
arg!(-x --exact "match exactly with the command name"),
381-
arg!(-F --pidfile <file> "read PIDs from file"),
382-
arg!(-L --logpidfile "fail if PID file is not locked"),
371+
// arg!(-F --pidfile <file> "read PIDs from file"),
372+
// arg!(-L --logpidfile "fail if PID file is not locked"),
383373
arg!(-r --runstates <state> "match runstates [D,S,Z,...]"),
384-
arg!(-A --"ignore-ancestors" "exclude our ancestors from results"),
385-
arg!(--cgroup <grp> "match by cgroup v2 names")
386-
.value_delimiter(','),
387-
arg!(--ns <PID> "match the processes that belong to the same namespace as <pid>"),
388-
arg!(--nslist <ns> "list which namespaces will be considered for the --ns option.")
389-
.value_delimiter(',')
390-
.value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]),
374+
// arg!(-A --"ignore-ancestors" "exclude our ancestors from results"),
375+
// arg!(--cgroup <grp> "match by cgroup v2 names")
376+
// .value_delimiter(','),
377+
// arg!(--ns <PID> "match the processes that belong to the same namespace as <pid>"),
378+
// arg!(--nslist <ns> "list which namespaces will be considered for the --ns option.")
379+
// .value_delimiter(',')
380+
// .value_parser(["ipc", "mnt", "net", "pid", "user", "uts"]),
391381
])
392382
.arg(
393383
Arg::new("pattern")

‎tests/by-util/test_pgrep.rs‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -347,3 +347,24 @@ fn test_parent_non_matching_parent() {
347347
.code_is(1)
348348
.no_output();
349349
}
350+
351+
#[test]
352+
#[cfg(target_os = "linux")]
353+
fn test_require_handler() {
354+
new_ucmd!()
355+
.arg("--require-handler")
356+
.arg("--signal=INT")
357+
.arg("NONEXISTENT")
358+
.fails()
359+
.no_output();
360+
}
361+
362+
#[test]
363+
#[cfg(target_os = "linux")]
364+
fn test_invalid_signal() {
365+
new_ucmd!()
366+
.arg("--signal=foo")
367+
.arg("NONEXISTENT")
368+
.fails()
369+
.stderr_contains("Unknown signal 'foo'");
370+
}

‎tests/by-util/test_pkill.rs‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,17 @@ fn test_invalid_arg() {
4444
new_ucmd!().arg("--definitely-invalid").fails().code_is(1);
4545
}
4646

47+
#[cfg(target_os = "linux")]
48+
#[test]
49+
fn test_inverse() {
50+
new_ucmd!()
51+
.arg("-0")
52+
.arg("--inverse")
53+
.arg("NONEXISTENT")
54+
.fails()
55+
.stderr_contains("Permission denied");
56+
}
57+
4758
#[cfg(unix)]
4859
#[test]
4960
fn test_help() {

0 commit comments

Comments
 (0)