From 473fccf421964b1573a88b2e7046f6dd996c5d74 Mon Sep 17 00:00:00 2001 From: Michal 'vorner' Vaner Date: Sat, 2 Jan 2021 17:56:29 +0100 Subject: [PATCH] Fold the signal-hook-sys inside the main signal-hook To avoid problems with extra dependency, compatibility, etc. --- .github/workflows/test.yaml | 1 + CHANGELOG.md | 2 + Cargo.lock | 10 +- Cargo.toml | 9 +- build.rs | 9 ++ signal-hook-async-std/Cargo.toml | 1 + signal-hook-mio/Cargo.toml | 1 + signal-hook-sys/Cargo.toml | 16 --- signal-hook-sys/LICENSE-APACHE | 1 - signal-hook-sys/LICENSE-MIT | 1 - signal-hook-sys/build.rs | 5 - signal-hook-sys/src/lib.rs | 116 ------------------ signal-hook-tokio/Cargo.toml | 1 + .../src => src/low_level}/extract.c | 4 + src/low_level/mod.rs | 2 +- src/low_level/siginfo.rs | 86 +++++++++++-- 16 files changed, 106 insertions(+), 159 deletions(-) create mode 100644 build.rs delete mode 100644 signal-hook-sys/Cargo.toml delete mode 120000 signal-hook-sys/LICENSE-APACHE delete mode 120000 signal-hook-sys/LICENSE-MIT delete mode 100644 signal-hook-sys/build.rs delete mode 100644 signal-hook-sys/src/lib.rs rename {signal-hook-sys/src => src/low_level}/extract.c (91%) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 9e523d1..a8677ec 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -151,6 +151,7 @@ jobs: - name: Check compilation run: | rm Cargo.lock + cargo update cargo check --no-default-features diff --git a/CHANGELOG.md b/CHANGELOG.md index 55de74e..b6587cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ * Allow extracting Origin from the raw `siginfo_t` structure by hand, without needing an iterator. +* Folding the signal-hook-sys inline (but still compiling C code only + conditionally). # 0.3.1 diff --git a/Cargo.lock b/Cargo.lock index 601eb66..d0ac113 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -859,10 +859,10 @@ dependencies = [ name = "signal-hook" version = "0.3.2" dependencies = [ + "cc", "libc", "serial_test", "signal-hook-registry 1.3.0", - "signal-hook-sys", ] [[package]] @@ -904,14 +904,6 @@ dependencies = [ "libc", ] -[[package]] -name = "signal-hook-sys" -version = "0.1.1" -dependencies = [ - "cc", - "libc", -] - [[package]] name = "signal-hook-tokio" version = "0.2.0" diff --git a/Cargo.toml b/Cargo.toml index abe74b0..c250dd2 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,8 @@ maintenance = { status = "actively-developed" } channel = [] default = ["channel", "iterator"] iterator = [] -extended-siginfo = ["channel", "iterator", "signal-hook-sys"] +extended-siginfo = ["channel", "iterator", "extended-siginfo-raw"] +extended-siginfo-raw = ["cc"] [workspace] members = [ @@ -29,16 +30,18 @@ members = [ "signal-hook-tokio", "signal-hook-mio", "signal-hook-async-std", - "signal-hook-sys", ] [dependencies] libc = "^0.2" signal-hook-registry = { version = "^1.3", path = "signal-hook-registry" } -signal-hook-sys = { version = "^0.1.1", path = "signal-hook-sys", optional = true } [dev-dependencies] serial_test = "^0.5" [package.metadata.docs.rs] all-features = true +rustdoc-args = ["--cfg", "docsrs"] + +[build-dependencies] +cc = { version = "^1", optional = true } diff --git a/build.rs b/build.rs new file mode 100644 index 0000000..82545f1 --- /dev/null +++ b/build.rs @@ -0,0 +1,9 @@ +#[cfg(feature = "extended-siginfo-raw")] +fn main() { + cc::Build::new() + .file("src/low_level/extract.c") + .compile("extract"); +} + +#[cfg(not(feature = "extended-siginfo-raw"))] +fn main() {} diff --git a/signal-hook-async-std/Cargo.toml b/signal-hook-async-std/Cargo.toml index ce3f21f..fc1f19f 100644 --- a/signal-hook-async-std/Cargo.toml +++ b/signal-hook-async-std/Cargo.toml @@ -30,3 +30,4 @@ serial_test = "~0.5" [package.metadata.docs.rs] all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/signal-hook-mio/Cargo.toml b/signal-hook-mio/Cargo.toml index 588517e..8453482 100644 --- a/signal-hook-mio/Cargo.toml +++ b/signal-hook-mio/Cargo.toml @@ -36,3 +36,4 @@ serial_test = "~0.5" [package.metadata.docs.rs] all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/signal-hook-sys/Cargo.toml b/signal-hook-sys/Cargo.toml deleted file mode 100644 index b5a0af5..0000000 --- a/signal-hook-sys/Cargo.toml +++ /dev/null @@ -1,16 +0,0 @@ -[package] -name = "signal-hook-sys" -version = "0.1.1" -authors = ["Michal 'vorner' Vaner "] -edition = "2018" -description = "Backend crate for signal-hook, with some low-level C code" -documentation = "https://docs.rs/signal-hook-sys" -license = "Apache-2.0/MIT" - -# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html - -[dependencies] -libc = "~0.2" - -[build-dependencies] -cc = "~1" diff --git a/signal-hook-sys/LICENSE-APACHE b/signal-hook-sys/LICENSE-APACHE deleted file mode 120000 index 965b606..0000000 --- a/signal-hook-sys/LICENSE-APACHE +++ /dev/null @@ -1 +0,0 @@ -../LICENSE-APACHE \ No newline at end of file diff --git a/signal-hook-sys/LICENSE-MIT b/signal-hook-sys/LICENSE-MIT deleted file mode 120000 index 76219eb..0000000 --- a/signal-hook-sys/LICENSE-MIT +++ /dev/null @@ -1 +0,0 @@ -../LICENSE-MIT \ No newline at end of file diff --git a/signal-hook-sys/build.rs b/signal-hook-sys/build.rs deleted file mode 100644 index bb4ce6d..0000000 --- a/signal-hook-sys/build.rs +++ /dev/null @@ -1,5 +0,0 @@ -use cc::Build; - -fn main() { - Build::new().file("src/extract.c").compile("extract"); -} diff --git a/signal-hook-sys/src/lib.rs b/signal-hook-sys/src/lib.rs deleted file mode 100644 index a3cdb4d..0000000 --- a/signal-hook-sys/src/lib.rs +++ /dev/null @@ -1,116 +0,0 @@ -//! Low-level internals of [`signal-hook`](https://docs.rs/signal-hook). -//! -//! This crate contains some internal APIs, split off to a separate crate for technical reasons. Do -//! not use directly. There are no stability guarantees, no documentation and you should use -//! `signal-hook` directly. - -#[doc(hidden)] -pub mod internal { - use libc::{pid_t, siginfo_t, uid_t}; - - // Careful: make sure the signature and the constants match the C source - extern "C" { - fn sighook_signal_cause(info: &siginfo_t) -> Cause; - fn sighook_signal_pid(info: &siginfo_t) -> pid_t; - fn sighook_signal_uid(info: &siginfo_t) -> uid_t; - } - - // Warning: must be in sync with the C code - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - #[non_exhaustive] - #[repr(u8)] - pub enum Cause { - Unknown = 0, - Kernel = 1, - User = 2, - TKill = 3, - Queue = 4, - MesgQ = 5, - Exited = 6, - Killed = 7, - Dumped = 8, - Trapped = 9, - Stopped = 10, - Continued = 11, - } - - impl Cause { - // The MacOs doesn't use the SI_* constants and leaves si_code at 0. But it doesn't use an - // union, it has a good-behaved struct with fields and therefore we *can* read the values, - // even though they'd contain nonsense (zeroes). We wipe that out later. - #[cfg(target_os = "macos")] - fn has_process(self) -> bool { - true - } - - #[cfg(not(target_os = "macos"))] - fn has_process(self) -> bool { - use Cause::*; - match self { - Unknown | Kernel => false, - User | TKill | Queue | MesgQ | Exited | Killed | Dumped | Trapped | Stopped - | Continued => true, - } - } - } - - #[derive(Copy, Clone, Debug, Eq, PartialEq)] - #[non_exhaustive] - pub struct Process { - pub pid: pid_t, - pub uid: uid_t, - } - - impl Process { - /** - * Extract the process information. - * - * # Safety - * - * The `info` must have a `si_code` corresponding to some situation that has the `si_pid` - * and `si_uid` filled in. - */ - unsafe fn extract(info: &siginfo_t) -> Self { - Self { - pid: sighook_signal_pid(info), - uid: sighook_signal_uid(info), - } - } - } - - #[derive(Clone, Debug, Eq, PartialEq)] - #[non_exhaustive] - pub struct SigInfo { - pub cause: Cause, - pub process: Option, - } - - impl SigInfo { - /// Extract the rust-style sig info. - /// - /// # Safety - /// - /// The `si_code` and `si_signo` needs to be properly set according to what fields are - /// actually available in the structure on systems where this is backed by an union (on the - /// C side). - /// - /// Note that kernel-provided value satisfies this. Care needs to be taken only if the - /// structure is constructed by hand. - pub unsafe fn extract(info: &siginfo_t) -> Self { - let cause = sighook_signal_cause(info); - let process = if cause.has_process() { - let process = Process::extract(info); - // On macos we don't have the si_code to go by, but we can go by the values being - // empty there. - if cfg!(target_os = "macos") && process.pid == 0 && process.uid == 0 { - None - } else { - Some(process) - } - } else { - None - }; - Self { cause, process } - } - } -} diff --git a/signal-hook-tokio/Cargo.toml b/signal-hook-tokio/Cargo.toml index 42c6e86..b49463b 100644 --- a/signal-hook-tokio/Cargo.toml +++ b/signal-hook-tokio/Cargo.toml @@ -37,3 +37,4 @@ serial_test = "~0.5" [package.metadata.docs.rs] all-features = true +rustdoc-args = ["--cfg", "docsrs"] diff --git a/signal-hook-sys/src/extract.c b/src/low_level/extract.c similarity index 91% rename from signal-hook-sys/src/extract.c rename to src/low_level/extract.c index 8f181d8..5f22615 100644 --- a/signal-hook-sys/src/extract.c +++ b/src/low_level/extract.c @@ -1,3 +1,7 @@ +/* + * Low-level extraction code to overcome rust's libc not having the best access + * to siginfo_t details. + */ #include #include #include diff --git a/src/low_level/mod.rs b/src/low_level/mod.rs index 4d83efa..ac31670 100644 --- a/src/low_level/mod.rs +++ b/src/low_level/mod.rs @@ -10,7 +10,7 @@ use libc::c_int; pub mod channel; #[cfg(not(windows))] pub mod pipe; -#[cfg(feature = "signal-hook-sys")] +#[cfg(feature = "extended-siginfo-raw")] pub mod siginfo; pub use signal_hook_registry::{register, unregister}; diff --git a/src/low_level/siginfo.rs b/src/low_level/siginfo.rs index 4366cf1..a42f1f9 100644 --- a/src/low_level/siginfo.rs +++ b/src/low_level/siginfo.rs @@ -3,7 +3,54 @@ //! See [`Origin`]. use libc::{c_int, pid_t, siginfo_t, uid_t}; -use signal_hook_sys::internal::{Cause as ICause, SigInfo}; +// Careful: make sure the signature and the constants match the C source +extern "C" { + fn sighook_signal_cause(info: &siginfo_t) -> ICause; + fn sighook_signal_pid(info: &siginfo_t) -> pid_t; + fn sighook_signal_uid(info: &siginfo_t) -> uid_t; +} + +// Warning: must be in sync with the C code +#[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[non_exhaustive] +#[repr(u8)] +// For some reason, the fact it comes from the C makes rustc emit warning that *some* of these are +// not constructed. No idea why only some of them. +#[allow(dead_code)] +enum ICause { + Unknown = 0, + Kernel = 1, + User = 2, + TKill = 3, + Queue = 4, + MesgQ = 5, + Exited = 6, + Killed = 7, + Dumped = 8, + Trapped = 9, + Stopped = 10, + Continued = 11, +} + +impl ICause { + // The MacOs doesn't use the SI_* constants and leaves si_code at 0. But it doesn't use an + // union, it has a good-behaved struct with fields and therefore we *can* read the values, + // even though they'd contain nonsense (zeroes). We wipe that out later. + #[cfg(target_os = "macos")] + fn has_process(self) -> bool { + true + } + + #[cfg(not(target_os = "macos"))] + fn has_process(self) -> bool { + use ICause::*; + match self { + Unknown | Kernel => false, + User | TKill | Queue | MesgQ | Exited | Killed | Dumped | Trapped | Stopped + | Continued => true, + } + } +} /// Information about process, as presented in the signal metadata. #[derive(Copy, Clone, Debug, Eq, PartialEq)] @@ -16,6 +63,23 @@ pub struct Process { pub uid: uid_t, } +impl Process { + /** + * Extract the process information. + * + * # Safety + * + * The `info` must have a `si_code` corresponding to some situation that has the `si_pid` + * and `si_uid` filled in. + */ + unsafe fn extract(info: &siginfo_t) -> Self { + Self { + pid: sighook_signal_pid(info), + uid: sighook_signal_uid(info), + } + } +} + /// The means by which a signal was sent by other process. #[derive(Copy, Clone, Debug, Eq, PartialEq)] #[non_exhaustive] @@ -154,14 +218,22 @@ impl Origin { /// The value passed by kernel satisfies this, care must be taken only when constructed /// manually. pub unsafe fn extract(info: &siginfo_t) -> Self { + let cause = sighook_signal_cause(info); + let process = if cause.has_process() { + let process = Process::extract(info); + // On macos we don't have the si_code to go by, but we can go by the values being + // empty there. + if cfg!(target_os = "macos") && process.pid == 0 && process.uid == 0 { + None + } else { + Some(process) + } + } else { + None + }; let signal = info.si_signo; - let extracted = SigInfo::extract(info); - let process = extracted.process.map(|p| Process { - pid: p.pid, - uid: p.uid, - }); Origin { - cause: extracted.cause.into(), + cause: cause.into(), signal, process, }