Skip to content

Commit 533edad

Browse files
authored
kill: replace nix::sys::signal with rustix::process (#12326)
1 parent abcdcdc commit 533edad

4 files changed

Lines changed: 143 additions & 25 deletions

File tree

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

src/uu/kill/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,8 @@ uucore = { workspace = true, features = ["signals"] }
2525
fluent = { workspace = true }
2626

2727
[target.'cfg(unix)'.dependencies]
28-
nix = { workspace = true, features = ["signal"] }
28+
libc = { workspace = true }
29+
rustix = { workspace = true, features = ["process"] }
2930

3031
[[bin]]
3132
name = "kill"

src/uu/kill/src/kill.rs

Lines changed: 93 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,19 +3,22 @@
33
// For the full copyright and license information, please view the LICENSE
44
// file that was distributed with this source code.
55

6-
// spell-checker:ignore (ToDO) signalname pids killpg
6+
// spell-checker:ignore (ToDO) signalname pids killpg NSIG
77

88
use clap::{Arg, ArgAction, Command};
9-
use nix::sys::signal::{self, Signal};
10-
use nix::unistd::Pid;
11-
use std::io::Error;
9+
use rustix::process::{
10+
Pid, Signal, kill_current_process_group, kill_process, kill_process_group,
11+
test_kill_current_process_group, test_kill_process, test_kill_process_group,
12+
};
13+
use std::cmp::Ordering;
14+
use std::io;
1215
use uucore::display::Quotable;
1316
use uucore::error::{FromIo, UResult, USimpleError};
1417
use uucore::translate;
1518

1619
use uucore::signals::{
1720
signal_by_name_or_value, signal_list_name_by_value, signal_list_value_by_name_or_number,
18-
signal_name_by_value, signal_number_upper_bound,
21+
signal_number_upper_bound,
1922
};
2023
use uucore::{format_usage, show};
2124

@@ -68,18 +71,6 @@ pub fn uumain(args: impl uucore::Args) -> UResult<()> {
6871
15_usize //SIGTERM
6972
};
7073

71-
let sig_name = signal_name_by_value(sig);
72-
// Signal does not support converting from EXIT
73-
// Instead, nix::signal::kill expects Option::None to properly handle EXIT
74-
let sig: Option<Signal> = if sig_name.is_some_and(|name| name == "EXIT") {
75-
None
76-
} else {
77-
let sig = (sig as i32)
78-
.try_into()
79-
.map_err(|e| Error::from_raw_os_error(e as i32))?;
80-
Some(sig)
81-
};
82-
8374
let pids = parse_pids(&pids_or_signals)?;
8475
if pids.is_empty() {
8576
Err(USimpleError::new(1, translate!("kill-error-no-process-id")))
@@ -226,6 +217,55 @@ fn list(signals: &Vec<String>) {
226217
}
227218
}
228219

220+
/// Convert a validated non-realtime signal number to a rustix [`Signal`].
221+
///
222+
/// # Safety (justification)
223+
///
224+
/// The caller must guarantee `sig > 0`. In this module that invariant is
225+
/// upheld by the control flow in [`kill()`], which routes `sig == 0` to the
226+
/// `test_kill_*` functions before reaching this helper.
227+
///
228+
/// Signal validity is ensured by [`signal_by_name_or_value`], which accepts:
229+
/// named and numeric signals in the supported platform range. Realtime signals
230+
/// are handled by [`raw_kill`] before this helper is called, because rustix
231+
/// does not permit libc-reserved realtime [`Signal`] values to be used for
232+
/// sending signals.
233+
fn sig_from_usize(sig: usize) -> Signal {
234+
debug_assert!(
235+
sig > 0,
236+
"signal 0 must be handled before calling this function"
237+
);
238+
debug_assert!(!is_realtime_signal(sig));
239+
// SAFETY: See function-level safety comment above.
240+
unsafe { Signal::from_raw_unchecked(sig as i32) }
241+
}
242+
243+
fn rustix_to_io(result: rustix::io::Result<()>) -> io::Result<()> {
244+
result.map_err(io::Error::from)
245+
}
246+
247+
fn raw_kill(pid: i32, sig: usize) -> io::Result<()> {
248+
let sig = i32::try_from(sig).map_err(|_| io::Error::from_raw_os_error(libc::EINVAL))?;
249+
if unsafe { libc::kill(pid as libc::pid_t, sig) } == 0 {
250+
Ok(())
251+
} else {
252+
Err(io::Error::last_os_error())
253+
}
254+
}
255+
256+
#[cfg(any(target_os = "linux", target_os = "android"))]
257+
fn is_realtime_signal(sig: usize) -> bool {
258+
let Ok(sig) = i32::try_from(sig) else {
259+
return false;
260+
};
261+
(libc::SIGRTMIN()..=libc::SIGRTMAX()).contains(&sig)
262+
}
263+
264+
#[cfg(not(any(target_os = "linux", target_os = "android")))]
265+
fn is_realtime_signal(_: usize) -> bool {
266+
false
267+
}
268+
229269
fn parse_signal_value(signal_name: &str) -> UResult<usize> {
230270
let optional_signal_value = signal_by_name_or_value(signal_name);
231271
match optional_signal_value {
@@ -250,13 +290,43 @@ fn parse_pids(pids: &[String]) -> UResult<Vec<i32>> {
250290
.collect()
251291
}
252292

253-
fn kill(sig: Option<Signal>, pids: &[i32]) {
293+
fn kill(sig: usize, pids: &[i32]) {
254294
for &pid in pids {
255-
if let Err(e) = signal::kill(Pid::from_raw(pid), sig) {
256-
show!(
257-
Error::from_raw_os_error(e as i32)
258-
.map_err_context(|| { translate!("kill-error-sending-signal", "pid" => pid) })
259-
);
295+
let result = match pid.cmp(&0) {
296+
Ordering::Equal if sig == 0 => rustix_to_io(test_kill_current_process_group()),
297+
Ordering::Equal if is_realtime_signal(sig) => raw_kill(0, sig),
298+
Ordering::Equal => rustix_to_io(kill_current_process_group(sig_from_usize(sig))),
299+
Ordering::Greater => {
300+
let pid = Pid::from_raw(pid).expect("pid > 0 guaranteed by Ordering::Greater");
301+
if sig == 0 {
302+
rustix_to_io(test_kill_process(pid))
303+
} else if is_realtime_signal(sig) {
304+
raw_kill(pid.as_raw_nonzero().get(), sig)
305+
} else {
306+
rustix_to_io(kill_process(pid, sig_from_usize(sig)))
307+
}
308+
}
309+
Ordering::Less => {
310+
let Some(abs_pid) = pid.checked_neg() else {
311+
show!(USimpleError::new(
312+
1,
313+
translate!("kill-error-sending-signal", "pid" => pid),
314+
));
315+
continue;
316+
};
317+
let pid =
318+
Pid::from_raw(abs_pid).expect("abs_pid > 0 since pid < 0 and pid != i32::MIN");
319+
if sig == 0 {
320+
rustix_to_io(test_kill_process_group(pid))
321+
} else if is_realtime_signal(sig) {
322+
raw_kill(-pid.as_raw_nonzero().get(), sig)
323+
} else {
324+
rustix_to_io(kill_process_group(pid, sig_from_usize(sig)))
325+
}
326+
}
327+
};
328+
if let Err(e) = result {
329+
show!(e.map_err_context(|| translate!("kill-error-sending-signal", "pid" => pid)));
260330
}
261331
}
262332
}

tests/by-util/test_kill.rs

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -493,3 +493,49 @@ fn test_kill_signal_only_no_pid() {
493493
.fails()
494494
.stderr_contains("no process ID specified");
495495
}
496+
497+
#[test]
498+
fn test_kill_signal_zero_process() {
499+
let target = Target::new();
500+
// kill -0 should succeed for a running process (signal 0 = existence check)
501+
new_ucmd!()
502+
.arg("-0")
503+
.arg(format!("{}", target.pid()))
504+
.succeeds();
505+
}
506+
507+
#[test]
508+
fn test_kill_signal_zero_new_form() {
509+
let target = Target::new();
510+
// kill -s 0 should also work
511+
new_ucmd!()
512+
.arg("-s")
513+
.arg("0")
514+
.arg(format!("{}", target.pid()))
515+
.succeeds();
516+
}
517+
518+
#[test]
519+
fn test_kill_signal_zero_nonexistent() {
520+
// kill -0 with a nonexistent PID should fail
521+
new_ucmd!().arg("-0").arg("999999999").fails();
522+
}
523+
524+
#[test]
525+
fn test_kill_signal_zero_current_process_group() {
526+
// kill -0 0 should succeed (checks current process group)
527+
new_ucmd!().arg("-0").arg("0").succeeds();
528+
}
529+
530+
#[cfg(any(target_os = "linux", target_os = "android"))]
531+
#[test]
532+
fn test_kill_realtime_signal() {
533+
let mut target = Target::new();
534+
// kill -s RTMIN should send SIGRTMIN and terminate the process
535+
new_ucmd!()
536+
.arg("-s")
537+
.arg("RTMIN")
538+
.arg(format!("{}", target.pid()))
539+
.succeeds();
540+
assert_eq!(target.wait_for_signal(), Some(libc::SIGRTMIN()));
541+
}

0 commit comments

Comments
 (0)