Documentation for sigaction function and related structs. Solves #319 #839
Conversation
src/sys/signal.rs
Outdated
| SigAction(extern fn(libc::c_int, *mut libc::siginfo_t, *mut libc::c_void)) | ||
| } | ||
|
|
||
| /// Struct holding data for [`sigaction`](fn.sigaction.html) function. | ||
| /// | ||
| /// `SigAction` holds the signal action itself (which can be ignoring, custom handler or a default |
There was a problem hiding this comment.
Get rid of everything within the parens.
src/sys/signal.rs
Outdated
| @@ -350,15 +350,38 @@ impl AsRef<libc::sigset_t> for SigSet { | |||
| } | |||
| } | |||
|
|
|||
| /// Enum holding signal action for [`sigaction`](fn.sigaction.html) function. | |||
There was a problem hiding this comment.
"Specifies the signal action for the ... function"
src/sys/signal.rs
Outdated
| @@ -350,15 +350,38 @@ impl AsRef<libc::sigset_t> for SigSet { | |||
| } | |||
| } | |||
|
|
|||
| /// Enum holding signal action for [`sigaction`](fn.sigaction.html) function. | |||
| /// | |||
| /// Signal can be ignored, may have a custom handler or default one. | |||
There was a problem hiding this comment.
This is redundant with the variants which have their own descriptions, delete thei sline.
src/sys/signal.rs
Outdated
| #[allow(unknown_lints)] | ||
| #[derive(Debug, Clone, Copy, PartialEq)] | ||
| pub enum SigHandler { | ||
| /// Signal action should be set to a default one. |
There was a problem hiding this comment.
"Specifies that the default action should be used"
src/sys/signal.rs
Outdated
| SigDfl, | ||
| /// Signal should be ignored. |
There was a problem hiding this comment.
Specifies that the signal should be ignored.
src/sys/signal.rs
Outdated
| /// | ||
| /// Custom signal handlers should call only | ||
| /// [signal-safe](https://wiki.sei.cmu.edu/confluence/display/c/SIG30-C.+Call+only+asynchronous-safe+functions+within+signal+handlers) | ||
| /// functions. Here is the [list](http://man7.org/linux/man-pages/man7/signal-safety.7.html) of |
There was a problem hiding this comment.
Are there lists specified for POSIX or any other platforms? We should try to provide docs for other platforms as well.
src/sys/signal.rs
Outdated
| /// signal-safe functions for Linux. Great care should be taken if the signal handler needs access | ||
| /// to global state. Global state used in signal handler must not be protected with | ||
| /// [`Mutex`](https://doc.rust-lang.org/std/sync/struct.Mutex.html), it is better to stick to | ||
| /// [`atomic`](https://doc.rust-lang.org/std/sync/atomic/index.html) module or to use |
src/sys/signal.rs
Outdated
| /// | ||
| /// Here is an example of building asynchronous-safe signal handler. We're building a function that | ||
| /// waits until `SIGINT` arrives and reports it. The problem is that reporting right from the | ||
| /// signal handler may be unsafe if reporting code calls allocator or locking functions. This might |
src/sys/signal.rs
Outdated
| /// static ref SIGNAL_SET_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); | ||
| /// } | ||
| /// | ||
| /// // The signal handler itself: it uses write() system call, which is signal-safe in Linux. |
There was a problem hiding this comment.
"it uses the write() system call"
src/sys/signal.rs
Outdated
| /// extern "C" fn sighandler(_: c_int) { | ||
| /// let buf: [u8; 1] = [0]; | ||
| /// unsafe { | ||
| /// // throw-away write since it is subject to race conditions with close |
|
@serejkus Mostly simple edits required before we can merge this; can you fix those and rebase? |
|
@serejkus Just pinging you on this. We're pretty close to wrapping this one up and merging! |
|
Sorry, got a lot of work on my day job, will try my best to fix these issues on a weekend |
…truct by incorporating attributes
|
Is there anything I can do here to get this across the line ? |
| /// Signal handler should report to a signal reaction handler via write to a pipe. | ||
| /// | ||
| /// ```no_run | ||
| /// #[macro_use] extern crate lazy_static; |
There was a problem hiding this comment.
These three lines are no longer needed in Rust edition 2018
| /// // We're not using Mutex<RawFd>, because calling lock/unlock in signal handler | ||
| /// // is an unsafe operation, but we need a lock to set up SIGNAL_FD. | ||
| /// lazy_static! { | ||
| /// static ref SIGNAL_SET_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); |
There was a problem hiding this comment.
If you don't take the lock when reading SIGNAL_FD, then there's no point in taking the lock when writing it. I advise deleting the mutex.
Sigaction docs with an example and links to docs and security best practicies. Solves #319 .