Skip to content

Commit 6100aee

Browse files
authored
refactor: replace unsafe libc calls with safe nix crate wrappers (#11156)
Replace direct unsafe libc function calls with safe nix crate wrappers for process management functions. This improves code safety and maintainability by leveraging nix's type-safe abstractions for system calls like geteuid, getpgrp, getsid, and signal handling. The changes maintain identical functionality while eliminating unsafe code blocks and providing better error handling.
1 parent 6de55b4 commit 6100aee

1 file changed

Lines changed: 32 additions & 32 deletions

File tree

src/uucore/src/lib/features/process.rs

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@
1010
use libc::{gid_t, pid_t, uid_t};
1111
#[cfg(not(target_os = "redox"))]
1212
use nix::errno::Errno;
13+
use nix::sys::signal::{self as nix_signal, SigHandler, Signal};
14+
use nix::unistd::Pid;
1315
use std::io;
1416
use std::process::Child;
1517
use std::process::ExitStatus;
@@ -18,37 +20,35 @@ use std::sync::atomic::AtomicBool;
1820
use std::thread;
1921
use std::time::{Duration, Instant};
2022

21-
// SAFETY: These functions always succeed and return simple integers.
22-
2323
/// `geteuid()` returns the effective user ID of the calling process.
2424
pub fn geteuid() -> uid_t {
25-
unsafe { libc::geteuid() }
25+
nix::unistd::geteuid().as_raw()
2626
}
2727

2828
/// `getpgrp()` returns the process group ID of the calling process.
29-
/// It is a trivial wrapper over libc::getpgrp to "hide" the unsafe
29+
/// It is a trivial wrapper over nix::unistd::getpgrp.
3030
pub fn getpgrp() -> pid_t {
31-
unsafe { libc::getpgrp() }
31+
nix::unistd::getpgrp().as_raw()
3232
}
3333

3434
/// `getegid()` returns the effective group ID of the calling process.
3535
pub fn getegid() -> gid_t {
36-
unsafe { libc::getegid() }
36+
nix::unistd::getegid().as_raw()
3737
}
3838

3939
/// `getgid()` returns the real group ID of the calling process.
4040
pub fn getgid() -> gid_t {
41-
unsafe { libc::getgid() }
41+
nix::unistd::getgid().as_raw()
4242
}
4343

4444
/// `getuid()` returns the real user ID of the calling process.
4545
pub fn getuid() -> uid_t {
46-
unsafe { libc::getuid() }
46+
nix::unistd::getuid().as_raw()
4747
}
4848

4949
/// `getpid()` returns the pid of the calling process.
5050
pub fn getpid() -> pid_t {
51-
unsafe { libc::getpid() }
51+
nix::unistd::getpid().as_raw()
5252
}
5353

5454
/// `getsid()` returns the session ID of the process with process ID pid.
@@ -67,12 +67,12 @@ pub fn getpid() -> pid_t {
6767
/// so some system such as redox doesn't supported.
6868
#[cfg(not(target_os = "redox"))]
6969
pub fn getsid(pid: i32) -> Result<pid_t, Errno> {
70-
let result = unsafe { libc::getsid(pid) };
71-
if result == -1 {
72-
Err(Errno::last())
70+
let pid = if pid == 0 {
71+
None
7372
} else {
74-
Ok(result)
75-
}
73+
Some(Pid::from_raw(pid))
74+
};
75+
nix::unistd::getsid(pid).map(Pid::as_raw)
7676
}
7777

7878
/// Missing methods for Child objects
@@ -97,11 +97,15 @@ pub trait ChildExt {
9797

9898
impl ChildExt for Child {
9999
fn send_signal(&mut self, signal: usize) -> io::Result<()> {
100-
if unsafe { libc::kill(self.id() as pid_t, signal as i32) } == 0 {
101-
Ok(())
100+
let pid = Pid::from_raw(self.id() as pid_t);
101+
let result = if signal == 0 {
102+
nix_signal::kill(pid, None)
102103
} else {
103-
Err(io::Error::last_os_error())
104-
}
104+
let signal = Signal::try_from(signal as i32)
105+
.map_err(|_| io::Error::from_raw_os_error(libc::EINVAL))?;
106+
nix_signal::kill(pid, Some(signal))
107+
};
108+
result.map_err(|e| io::Error::from_raw_os_error(e as i32))
105109
}
106110

107111
fn send_signal_group(&mut self, signal: usize) -> io::Result<()> {
@@ -114,24 +118,20 @@ impl ChildExt for Child {
114118
// Signal 0 is special - it just checks if process exists, doesn't send anything.
115119
// No need to manipulate signal handlers for it.
116120
if signal == 0 {
117-
let result = unsafe { libc::kill(0, 0) };
118-
return if result == 0 {
119-
Ok(())
120-
} else {
121-
Err(io::Error::last_os_error())
122-
};
121+
return nix_signal::kill(Pid::from_raw(0), None)
122+
.map_err(|e| io::Error::from_raw_os_error(e as i32));
123123
}
124124

125+
let signal = Signal::try_from(signal as i32)
126+
.map_err(|_| io::Error::from_raw_os_error(libc::EINVAL))?;
127+
125128
// Ignore the signal temporarily so we don't receive it ourselves.
126-
let old_handler = unsafe { libc::signal(signal as i32, libc::SIG_IGN) };
127-
let result = unsafe { libc::kill(0, signal as i32) };
129+
let old_handler = unsafe { nix_signal::signal(signal, SigHandler::SigIgn) }
130+
.map_err(|e| io::Error::from_raw_os_error(e as i32))?;
131+
let result = nix_signal::kill(Pid::from_raw(0), Some(signal));
128132
// Restore the old handler
129-
unsafe { libc::signal(signal as i32, old_handler) };
130-
if result == 0 {
131-
Ok(())
132-
} else {
133-
Err(io::Error::last_os_error())
134-
}
133+
let _ = unsafe { nix_signal::signal(signal, old_handler) };
134+
result.map_err(|e| io::Error::from_raw_os_error(e as i32))
135135
}
136136

137137
fn wait_or_timeout(

0 commit comments

Comments
 (0)