-
Notifications
You must be signed in to change notification settings - Fork 684
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Signal enum #362
Signal enum #362
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,11 +2,60 @@ | |
// See http://rust-lang.org/COPYRIGHT. | ||
|
||
use libc; | ||
use {Errno, Result}; | ||
use {Errno, Error, Result}; | ||
use std::mem; | ||
use std::ptr; | ||
|
||
pub use libc::{ | ||
// Currently there is only one definition of c_int in libc, as well as only one | ||
// type for signal constants. | ||
// We would prefer to use the libc::c_int alias in the repr attribute. Unfortunately | ||
// this is not (yet) possible. | ||
#[derive(Clone, Copy, Debug, Eq, PartialEq)] | ||
#[repr(i32)] | ||
pub enum Signal { | ||
SIGHUP = libc::SIGHUP, | ||
SIGINT = libc::SIGINT, | ||
SIGQUIT = libc::SIGQUIT, | ||
SIGILL = libc::SIGILL, | ||
SIGTRAP = libc::SIGTRAP, | ||
SIGABRT = libc::SIGABRT, | ||
SIGBUS = libc::SIGBUS, | ||
SIGFPE = libc::SIGFPE, | ||
SIGKILL = libc::SIGKILL, | ||
SIGUSR1 = libc::SIGUSR1, | ||
SIGSEGV = libc::SIGSEGV, | ||
SIGUSR2 = libc::SIGUSR2, | ||
SIGPIPE = libc::SIGPIPE, | ||
SIGALRM = libc::SIGALRM, | ||
SIGTERM = libc::SIGTERM, | ||
#[cfg(not(target_os = "macos"))] | ||
SIGSTKFLT = libc::SIGSTKFLT, | ||
SIGCHLD = libc::SIGCHLD, | ||
SIGCONT = libc::SIGCONT, | ||
SIGSTOP = libc::SIGSTOP, | ||
SIGTSTP = libc::SIGTSTP, | ||
SIGTTIN = libc::SIGTTIN, | ||
SIGTTOU = libc::SIGTTOU, | ||
SIGURG = libc::SIGURG, | ||
SIGXCPU = libc::SIGXCPU, | ||
SIGXFSZ = libc::SIGXFSZ, | ||
SIGVTALRM = libc::SIGVTALRM, | ||
SIGPROF = libc::SIGPROF, | ||
SIGWINCH = libc::SIGWINCH, | ||
SIGIO = libc::SIGIO, | ||
#[cfg(not(target_os = "macos"))] | ||
SIGPWR = libc::SIGPWR, | ||
SIGSYS = libc::SIGSYS, | ||
#[cfg(target_os = "macos")] | ||
SIGEMT = libc::SIGEMT, | ||
#[cfg(target_os = "macos")] | ||
SIGINFO = libc::SIGINFO, | ||
} | ||
|
||
pub use self::Signal::*; | ||
|
||
#[cfg(not(target_os = "macos"))] | ||
const SIGNALS: [Signal; 31] = [ | ||
SIGHUP, | ||
SIGINT, | ||
SIGQUIT, | ||
|
@@ -22,6 +71,7 @@ pub use libc::{ | |
SIGPIPE, | ||
SIGALRM, | ||
SIGTERM, | ||
SIGSTKFLT, | ||
SIGCHLD, | ||
SIGCONT, | ||
SIGSTOP, | ||
|
@@ -35,26 +85,83 @@ pub use libc::{ | |
SIGPROF, | ||
SIGWINCH, | ||
SIGIO, | ||
SIGSYS, | ||
}; | ||
|
||
SIGPWR, | ||
SIGSYS]; | ||
#[cfg(target_os = "macos")] | ||
pub use libc::{ | ||
const SIGNALS: [Signal; 31] = [ | ||
SIGHUP, | ||
SIGINT, | ||
SIGQUIT, | ||
SIGILL, | ||
SIGTRAP, | ||
SIGABRT, | ||
SIGBUS, | ||
SIGFPE, | ||
SIGKILL, | ||
SIGUSR1, | ||
SIGSEGV, | ||
SIGUSR2, | ||
SIGPIPE, | ||
SIGALRM, | ||
SIGTERM, | ||
SIGCHLD, | ||
SIGCONT, | ||
SIGSTOP, | ||
SIGTSTP, | ||
SIGTTIN, | ||
SIGTTOU, | ||
SIGURG, | ||
SIGXCPU, | ||
SIGXFSZ, | ||
SIGVTALRM, | ||
SIGPROF, | ||
SIGWINCH, | ||
SIGIO, | ||
SIGSYS, | ||
SIGEMT, | ||
SIGINFO, | ||
}; | ||
|
||
#[cfg(not(target_os = "macos"))] | ||
pub use libc::{ | ||
SIGPWR, | ||
SIGSTKFLT, | ||
SIGIOT, // Alias for SIGABRT | ||
SIGPOLL, // Alias for SIGIO | ||
SIGUNUSED, // Alias for 31 | ||
}; | ||
SIGINFO]; | ||
|
||
pub const NSIG: libc::c_int = 32; | ||
|
||
pub struct SignalIterator { | ||
next: usize, | ||
} | ||
|
||
impl Iterator for SignalIterator { | ||
type Item = Signal; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I considered a nother method of iterating over enumerations. It uses an array of all elements and uses the slice module to produce and iterator. I think the following is a little clearer than an array which contains all the elements of the enumeration again. In particular, when the elements of the enumeration differ between platforms. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this approach is better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have two problems with this: Its slower. It cannot be used with enumerations whose variant representation dont form a single interval in the natural numbers. Therefore, I actually prefer the other approach, even though it is more verbose and probably makes the executable slightly bigger. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. After my last change it is no longer slower. However, we still cannot use its pattern for non consecutive enums. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. perhaps it's better to always have a static array of all the enum variants after all? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the concern is returning a slice iterator, we could newtype it to make it opaque. that would simplify implementation and work for non-contiguous enums. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It probably is. I'll change it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why would returning a slice iterator be a problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's part of the public interface, so a change to the implementation would be breaking. (my kingdom for an There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So we should export our own iterator. |
||
|
||
fn next(&mut self) -> Option<Signal> { | ||
if self.next < SIGNALS.len() { | ||
let next_signal = SIGNALS[self.next]; | ||
self.next += 1; | ||
Some(next_signal) | ||
} else { | ||
None | ||
} | ||
} | ||
} | ||
|
||
impl Signal { | ||
pub fn iterator() -> SignalIterator { | ||
SignalIterator{next: 0} | ||
} | ||
|
||
// We do not implement the From trait, because it is supposed to be infallible. | ||
// With Rust RFC 1542 comes the appropriate trait TryFrom. Once it is | ||
// implemented, we'll replace this function. | ||
#[inline] | ||
pub fn from_c_int(signum: libc::c_int) -> Result<Signal> { | ||
match 0 < signum && signum < NSIG { | ||
true => Ok(unsafe { mem::transmute(signum) }), | ||
false => Err(Error::invalid_argument()), | ||
} | ||
} | ||
} | ||
|
||
pub const SIGIOT : Signal = SIGABRT; | ||
pub const SIGPOLL : Signal = SIGIO; | ||
pub const SIGUNUSED : Signal = SIGSYS; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. are these aliases cross-platform? |
||
|
||
bitflags!{ | ||
flags SaFlags: libc::c_int { | ||
const SA_NOCLDSTOP = libc::SA_NOCLDSTOP, | ||
|
@@ -80,7 +187,6 @@ pub struct SigSet { | |
sigset: libc::sigset_t | ||
} | ||
|
||
pub type SigNum = libc::c_int; | ||
|
||
impl SigSet { | ||
pub fn all() -> SigSet { | ||
|
@@ -97,40 +203,33 @@ impl SigSet { | |
SigSet { sigset: sigset } | ||
} | ||
|
||
pub fn add(&mut self, signum: SigNum) -> Result<()> { | ||
let res = unsafe { libc::sigaddset(&mut self.sigset as *mut libc::sigset_t, signum) }; | ||
|
||
Errno::result(res).map(drop) | ||
pub fn add(&mut self, signal: Signal) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think we should rename this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could do that. Though, I would want to do that in a separate PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok with pushing that change to a later PR. This one can just be the type changing to an enum. |
||
unsafe { libc::sigaddset(&mut self.sigset as *mut libc::sigset_t, signal as libc::c_int) }; | ||
} | ||
|
||
pub fn clear(&mut self) -> Result<()> { | ||
let res = unsafe { libc::sigemptyset(&mut self.sigset as *mut libc::sigset_t) }; | ||
|
||
Errno::result(res).map(drop) | ||
pub fn clear(&mut self) { | ||
unsafe { libc::sigemptyset(&mut self.sigset as *mut libc::sigset_t) }; | ||
} | ||
|
||
pub fn remove(&mut self, signum: SigNum) -> Result<()> { | ||
let res = unsafe { libc::sigdelset(&mut self.sigset as *mut libc::sigset_t, signum) }; | ||
|
||
Errno::result(res).map(drop) | ||
pub fn remove(&mut self, signal: Signal) { | ||
unsafe { libc::sigdelset(&mut self.sigset as *mut libc::sigset_t, signal as libc::c_int) }; | ||
} | ||
|
||
pub fn extend(&mut self, other: &SigSet) -> Result<()> { | ||
for i in 1..NSIG { | ||
if try!(other.contains(i)) { | ||
try!(self.add(i)); | ||
} | ||
pub fn contains(&self, signal: Signal) -> bool { | ||
let res = unsafe { libc::sigismember(&self.sigset as *const libc::sigset_t, signal as libc::c_int) }; | ||
|
||
match res { | ||
1 => true, | ||
0 => false, | ||
_ => unreachable!("unexpected value from sigismember"), | ||
} | ||
Ok(()) | ||
} | ||
|
||
pub fn contains(&self, signum: SigNum) -> Result<bool> { | ||
let res = unsafe { libc::sigismember(&self.sigset as *const libc::sigset_t, signum) }; | ||
|
||
match try!(Errno::result(res)) { | ||
1 => Ok(true), | ||
0 => Ok(false), | ||
_ => unreachable!("unexpected value from sigismember"), | ||
pub fn extend(&mut self, other: &SigSet) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. later PR could then also make this take |
||
for signal in Signal::iterator() { | ||
if other.contains(signal) { | ||
self.add(signal); | ||
} | ||
} | ||
} | ||
|
||
|
@@ -165,11 +264,11 @@ impl SigSet { | |
|
||
/// Suspends execution of the calling thread until one of the signals in the | ||
/// signal mask becomes pending, and returns the accepted signal. | ||
pub fn wait(&self) -> Result<SigNum> { | ||
let mut signum: SigNum = unsafe { mem::uninitialized() }; | ||
pub fn wait(&self) -> Result<Signal> { | ||
let mut signum: libc::c_int = unsafe { mem::uninitialized() }; | ||
let res = unsafe { libc::sigwait(&self.sigset as *const libc::sigset_t, &mut signum) }; | ||
|
||
Errno::result(res).map(|_| signum) | ||
Errno::result(res).map(|_| Signal::from_c_int(signum).unwrap()) | ||
} | ||
} | ||
|
||
|
@@ -185,8 +284,8 @@ impl AsRef<libc::sigset_t> for SigSet { | |
pub enum SigHandler { | ||
SigDfl, | ||
SigIgn, | ||
Handler(extern fn(SigNum)), | ||
SigAction(extern fn(SigNum, *mut libc::siginfo_t, *mut libc::c_void)) | ||
Handler(extern fn(libc::c_int)), | ||
SigAction(extern fn(libc::c_int, *mut libc::siginfo_t, *mut libc::c_void)) | ||
} | ||
|
||
pub struct SigAction { | ||
|
@@ -214,11 +313,11 @@ impl SigAction { | |
} | ||
} | ||
|
||
pub unsafe fn sigaction(signum: SigNum, sigaction: &SigAction) -> Result<SigAction> { | ||
pub unsafe fn sigaction(signal: Signal, sigaction: &SigAction) -> Result<SigAction> { | ||
let mut oldact = mem::uninitialized::<libc::sigaction>(); | ||
|
||
let res = | ||
libc::sigaction(signum, &sigaction.sigaction as *const libc::sigaction, &mut oldact as *mut libc::sigaction); | ||
libc::sigaction(signal as libc::c_int, &sigaction.sigaction as *const libc::sigaction, &mut oldact as *mut libc::sigaction); | ||
|
||
Errno::result(res).map(|_| SigAction { sigaction: oldact }) | ||
} | ||
|
@@ -257,14 +356,14 @@ pub fn pthread_sigmask(how: SigFlags, | |
Errno::result(res).map(drop) | ||
} | ||
|
||
pub fn kill(pid: libc::pid_t, signum: SigNum) -> Result<()> { | ||
let res = unsafe { libc::kill(pid, signum) }; | ||
pub fn kill(pid: libc::pid_t, signal: Signal) -> Result<()> { | ||
let res = unsafe { libc::kill(pid, signal as libc::c_int) }; | ||
|
||
Errno::result(res).map(drop) | ||
} | ||
|
||
pub fn raise(signum: SigNum) -> Result<()> { | ||
let res = unsafe { libc::raise(signum) }; | ||
pub fn raise(signal: Signal) -> Result<()> { | ||
let res = unsafe { libc::raise(signal as libc::c_int) }; | ||
|
||
Errno::result(res).map(drop) | ||
} | ||
|
@@ -276,70 +375,70 @@ mod tests { | |
#[test] | ||
fn test_contains() { | ||
let mut mask = SigSet::empty(); | ||
mask.add(SIGUSR1).unwrap(); | ||
mask.add(SIGUSR1); | ||
|
||
assert_eq!(mask.contains(SIGUSR1), Ok(true)); | ||
assert_eq!(mask.contains(SIGUSR2), Ok(false)); | ||
assert!(mask.contains(SIGUSR1)); | ||
assert!(!mask.contains(SIGUSR2)); | ||
|
||
let all = SigSet::all(); | ||
assert_eq!(all.contains(SIGUSR1), Ok(true)); | ||
assert_eq!(all.contains(SIGUSR2), Ok(true)); | ||
assert!(all.contains(SIGUSR1)); | ||
assert!(all.contains(SIGUSR2)); | ||
} | ||
|
||
#[test] | ||
fn test_clear() { | ||
let mut set = SigSet::all(); | ||
set.clear().unwrap(); | ||
for i in 1..NSIG { | ||
assert_eq!(set.contains(i), Ok(false)); | ||
set.clear(); | ||
for signal in Signal::iterator() { | ||
assert!(!set.contains(signal)); | ||
} | ||
} | ||
|
||
#[test] | ||
fn test_extend() { | ||
let mut one_signal = SigSet::empty(); | ||
one_signal.add(SIGUSR1).unwrap(); | ||
one_signal.add(SIGUSR1); | ||
|
||
let mut two_signals = SigSet::empty(); | ||
two_signals.add(SIGUSR2).unwrap(); | ||
two_signals.extend(&one_signal).unwrap(); | ||
two_signals.add(SIGUSR2); | ||
two_signals.extend(&one_signal); | ||
|
||
assert_eq!(two_signals.contains(SIGUSR1), Ok(true)); | ||
assert_eq!(two_signals.contains(SIGUSR2), Ok(true)); | ||
assert!(two_signals.contains(SIGUSR1)); | ||
assert!(two_signals.contains(SIGUSR2)); | ||
} | ||
|
||
#[test] | ||
fn test_thread_signal_block() { | ||
let mut mask = SigSet::empty(); | ||
mask.add(SIGUSR1).unwrap(); | ||
mask.add(SIGUSR1); | ||
|
||
assert!(mask.thread_block().is_ok()); | ||
} | ||
|
||
#[test] | ||
fn test_thread_signal_swap() { | ||
let mut mask = SigSet::empty(); | ||
mask.add(SIGUSR1).unwrap(); | ||
mask.add(SIGUSR1); | ||
mask.thread_block().unwrap(); | ||
|
||
assert!(SigSet::thread_get_mask().unwrap().contains(SIGUSR1).unwrap()); | ||
assert!(SigSet::thread_get_mask().unwrap().contains(SIGUSR1)); | ||
|
||
let mask2 = SigSet::empty(); | ||
mask.add(SIGUSR2).unwrap(); | ||
mask.add(SIGUSR2); | ||
|
||
let oldmask = mask2.thread_swap_mask(SIG_SETMASK).unwrap(); | ||
|
||
assert!(oldmask.contains(SIGUSR1).unwrap()); | ||
assert!(!oldmask.contains(SIGUSR2).unwrap()); | ||
assert!(oldmask.contains(SIGUSR1)); | ||
assert!(!oldmask.contains(SIGUSR2)); | ||
} | ||
|
||
// TODO(#251): Re-enable after figuring out flakiness. | ||
#[cfg(not(any(target_os = "macos", target_os = "ios")))] | ||
#[test] | ||
fn test_sigwait() { | ||
let mut mask = SigSet::empty(); | ||
mask.add(SIGUSR1).unwrap(); | ||
mask.add(SIGUSR2).unwrap(); | ||
mask.add(SIGUSR1); | ||
mask.add(SIGUSR2); | ||
mask.thread_block().unwrap(); | ||
|
||
raise(SIGUSR1).unwrap(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The following type and function are only needed internally. They are used in the implementation of the iterator and in
wait.rs
. Because of the latter I hade to make them public (for now).I propose putting the whole code into a private module (src/internal/sys/signal.rs) and then reexport what is supposed to be public here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For hiding, your proposal is the right thing to do. Though there's no need to put everything in that module I think: circular dependencies among modules in a crate are ok. You could have an internal module depend on this module, and define
signal_from
. Then this module andwait
could both depend on that module.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll implement that.