From 0c6d5765f674dec54d53c5afd0b0c83603864ff2 Mon Sep 17 00:00:00 2001 From: Alan Somers Date: Mon, 9 Aug 2021 13:28:43 -0600 Subject: [PATCH] Optionally implement TryFrom in libc_enum! This saves code in several separate places that need to do this separately. At the same time, remove a few uses of mem::transmute that were implementing TryFrom or similar functionality. Issue #373 --- CHANGELOG.md | 11 ++++ src/macros.rs | 128 +++++++++++++++++++++++++++++++++++++++++---- src/sys/event.rs | 24 +++++++-- src/sys/signal.rs | 15 +----- src/sys/termios.rs | 119 ++--------------------------------------- 5 files changed, 153 insertions(+), 144 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d311d14460..1faf8f9777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -43,17 +43,28 @@ This project adheres to [Semantic Versioning](https://semver.org/). - Many more functions, mostly contructors, are now `const`. (#[1476](https://github.com/nix-rust/nix/pull/1476)) +- `sys::event::KEvent::filter` now returns a `Result` instead of being + infalliable. The only cases where it will now return an error are cases + where it previously would've had undefined behavior. + (#[1484](https://github.com/nix-rust/nix/pull/1484)) + ### Fixed - Added more errno definitions for better backwards compatibility with Nix 0.21.0. (#[1467](https://github.com/nix-rust/nix/pull/1467)) +- Fixed potential undefined behavior in `Signal::try_from` on some platforms. + (#[1484](https://github.com/nix-rust/nix/pull/1484)) + ### Removed - Removed a couple of termios constants on redox that were never actually supported. (#[1483](https://github.com/nix-rust/nix/pull/1483)) +- Removed `nix::sys::signal::NSIG`. It was of dubious utility, and not correct + for all platforms. + (#[1484](https://github.com/nix-rust/nix/pull/1484)) ## [0.22.0] - 9 July 2021 ### Added diff --git a/src/macros.rs b/src/macros.rs index 7d6ac8dfbf..927e7e7c76 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -83,9 +83,9 @@ macro_rules! libc_bitflags { macro_rules! libc_enum { // Exit rule. (@make_enum + name: $BitFlags:ident, { $v:vis - name: $BitFlags:ident, attrs: [$($attrs:tt)*], entries: [$($entries:tt)*], } @@ -97,38 +97,98 @@ macro_rules! libc_enum { } }; + // Exit rule including TryFrom + (@make_enum + name: $BitFlags:ident, + { + $v:vis + attrs: [$($attrs:tt)*], + entries: [$($entries:tt)*], + from_type: $repr:path, + try_froms: [$($try_froms:tt)*] + } + ) => { + $($attrs)* + #[derive(Clone, Copy, Debug, Eq, Hash, Ord, PartialEq, PartialOrd)] + $v enum $BitFlags { + $($entries)* + } + #[allow(unused_doc_comment)] + impl ::std::convert::TryFrom<$repr> for $BitFlags { + type Error = $crate::Error; + fn try_from(x: $repr) -> $crate::Result { + match x { + $($try_froms)* + _ => Err($crate::Error::EINVAL) + } + } + } + }; + // Done accumulating. (@accumulate_entries + name: $BitFlags:ident, + { + $v:vis + attrs: $attrs:tt, + }, + $entries:tt, + $try_froms:tt; + ) => { + libc_enum! { + @make_enum + name: $BitFlags, + { + $v + attrs: $attrs, + entries: $entries, + } + } + }; + + // Done accumulating and want TryFrom + (@accumulate_entries + name: $BitFlags:ident, { $v:vis - name: $BitFlags:ident, attrs: $attrs:tt, + from_type: $repr:path, }, - $entries:tt; + $entries:tt, + $try_froms:tt; ) => { libc_enum! { @make_enum + name: $BitFlags, { $v - name: $BitFlags, attrs: $attrs, entries: $entries, + from_type: $repr, + try_froms: $try_froms } } }; // Munch an attr. (@accumulate_entries + name: $BitFlags:ident, $prefix:tt, - [$($entries:tt)*]; + [$($entries:tt)*], + [$($try_froms:tt)*]; #[$attr:meta] $($tail:tt)* ) => { libc_enum! { @accumulate_entries + name: $BitFlags, $prefix, [ $($entries)* #[$attr] + ], + [ + $($try_froms)* + #[$attr] ]; $($tail)* } @@ -136,32 +196,47 @@ macro_rules! libc_enum { // Munch last ident if not followed by a comma. (@accumulate_entries + name: $BitFlags:ident, $prefix:tt, - [$($entries:tt)*]; + [$($entries:tt)*], + [$($try_froms:tt)*]; $entry:ident ) => { libc_enum! { @accumulate_entries + name: $BitFlags, $prefix, [ $($entries)* $entry = libc::$entry, + ], + [ + $($try_froms)* + libc::$entry => Ok($BitFlags::$entry), ]; } }; // Munch an ident; covers terminating comma case. (@accumulate_entries + name: $BitFlags:ident, $prefix:tt, - [$($entries:tt)*]; - $entry:ident, $($tail:tt)* + [$($entries:tt)*], + [$($try_froms:tt)*]; + $entry:ident, + $($tail:tt)* ) => { libc_enum! { @accumulate_entries + name: $BitFlags, $prefix, [ $($entries)* $entry = libc::$entry, + ], + [ + $($try_froms)* + libc::$entry => Ok($BitFlags::$entry), ]; $($tail)* } @@ -169,16 +244,24 @@ macro_rules! libc_enum { // Munch an ident and cast it to the given type; covers terminating comma. (@accumulate_entries + name: $BitFlags:ident, $prefix:tt, - [$($entries:tt)*]; - $entry:ident as $ty:ty, $($tail:tt)* + [$($entries:tt)*], + [$($try_froms:tt)*]; + $entry:ident as $ty:ty, + $($tail:tt)* ) => { libc_enum! { @accumulate_entries + name: $BitFlags, $prefix, [ $($entries)* $entry = libc::$entry as $ty, + ], + [ + $($try_froms)* + libc::$entry as $ty => Ok($BitFlags::$entry), ]; $($tail)* } @@ -193,11 +276,34 @@ macro_rules! libc_enum { ) => { libc_enum! { @accumulate_entries + name: $BitFlags, + { + $v + attrs: [$(#[$attr])*], + }, + [], + []; + $($vals)* + } + }; + + // Entry rule including TryFrom + ( + $(#[$attr:meta])* + $v:vis enum $BitFlags:ident { + $($vals:tt)* + } + impl TryFrom<$repr:path> + ) => { + libc_enum! { + @accumulate_entries + name: $BitFlags, { $v - name: $BitFlags, attrs: [$(#[$attr])*], + from_type: $repr, }, + [], []; $($vals)* } diff --git a/src/sys/event.rs b/src/sys/event.rs index 1f4c1b4185..a5fe885f0c 100644 --- a/src/sys/event.rs +++ b/src/sys/event.rs @@ -6,9 +6,9 @@ use crate::{Errno, Result}; use libc::{timespec, time_t, c_int, c_long, intptr_t, uintptr_t}; #[cfg(target_os = "netbsd")] use libc::{timespec, time_t, c_long, intptr_t, uintptr_t, size_t}; +use std::convert::TryInto; use std::os::unix::io::RawFd; use std::ptr; -use std::mem; // Redefine kevent in terms of programmer-friendly enums and bitfields. #[repr(C)] @@ -76,6 +76,7 @@ libc_enum! { EVFILT_VNODE, EVFILT_WRITE, } + impl TryFrom } #[cfg(any(target_os = "dragonfly", target_os = "freebsd", @@ -233,8 +234,8 @@ impl KEvent { self.kevent.ident } - pub fn filter(&self) -> EventFilter { - unsafe { mem::transmute(self.kevent.filter as type_of_event_filter) } + pub fn filter(&self) -> Result { + self.kevent.filter.try_into() } pub fn flags(&self) -> EventFlag { @@ -313,6 +314,8 @@ pub fn ev_set(ev: &mut KEvent, #[test] fn test_struct_kevent() { + use std::mem; + let udata : intptr_t = 12345; let actual = KEvent::new(0xdead_beef, @@ -322,10 +325,23 @@ fn test_struct_kevent() { 0x1337, udata); assert_eq!(0xdead_beef, actual.ident()); - assert_eq!(libc::EVFILT_READ, actual.filter() as type_of_event_filter); + assert_eq!(libc::EVFILT_READ, actual.kevent.filter); assert_eq!(libc::EV_ONESHOT | libc::EV_ADD, actual.flags().bits()); assert_eq!(libc::NOTE_CHILD | libc::NOTE_EXIT, actual.fflags().bits()); assert_eq!(0x1337, actual.data() as type_of_data); assert_eq!(udata as type_of_udata, actual.udata() as type_of_udata); assert_eq!(mem::size_of::(), mem::size_of::()); } + +#[test] +fn test_kevent_filter() { + let udata : intptr_t = 12345; + + let actual = KEvent::new(0xdead_beef, + EventFilter::EVFILT_READ, + EventFlag::EV_ONESHOT | EventFlag::EV_ADD, + FilterFlag::NOTE_CHILD | FilterFlag::NOTE_EXIT, + 0x1337, + udata); + assert_eq!(EventFilter::EVFILT_READ, actual.filter().unwrap()); +} diff --git a/src/sys/signal.rs b/src/sys/signal.rs index 0911cfaa2d..021abd4b91 100644 --- a/src/sys/signal.rs +++ b/src/sys/signal.rs @@ -71,6 +71,7 @@ libc_enum!{ target_os = "redox")))] SIGINFO, } + impl TryFrom } impl FromStr for Signal { @@ -335,8 +336,6 @@ const SIGNALS: [Signal; 31] = [ SIGEMT, SIGINFO]; -pub const NSIG: libc::c_int = 32; - #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)] pub struct SignalIterator { next: usize, @@ -362,18 +361,6 @@ impl Signal { } } -impl TryFrom for Signal { - type Error = Error; - - fn try_from(signum: libc::c_int) -> Result { - if 0 < signum && signum < NSIG { - Ok(unsafe { mem::transmute(signum) }) - } else { - Err(Error::from(Errno::EINVAL)) - } - } -} - pub const SIGIOT : Signal = SIGABRT; pub const SIGPOLL : Signal = SIGIO; pub const SIGUNUSED : Signal = SIGSYS; diff --git a/src/sys/termios.rs b/src/sys/termios.rs index 1e4307e3ec..01d4608039 100644 --- a/src/sys/termios.rs +++ b/src/sys/termios.rs @@ -152,11 +152,11 @@ //! # } //! ``` use cfg_if::cfg_if; -use crate::{Error, Result}; +use crate::Result; use crate::errno::Errno; use libc::{self, c_int, tcflag_t}; use std::cell::{Ref, RefCell}; -use std::convert::{From, TryFrom}; +use std::convert::From; use std::mem; use std::os::unix::io::RawFd; @@ -340,119 +340,7 @@ libc_enum!{ #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] B4000000, } -} - -impl TryFrom for BaudRate { - type Error = Error; - - fn try_from(s: libc::speed_t) -> Result { - use libc::{B0, B50, B75, B110, B134, B150, B200, B300, B600, B1200, B1800, B2400, B4800, - B9600, B19200, B38400, B57600, B115200, B230400}; - #[cfg(any(target_os = "android", target_os = "linux"))] - use libc::{B500000, B576000, B1000000, B1152000, B1500000, B2000000}; - #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] - use libc::{B2500000, B3000000, B3500000, B4000000}; - #[cfg(any(target_os = "dragonfly", - target_os = "freebsd", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd"))] - use libc::{B7200, B14400, B28800, B76800}; - #[cfg(any(target_os = "android", - target_os = "freebsd", - target_os = "linux", - target_os = "netbsd"))] - use libc::{B460800, B921600}; - #[cfg(any(target_os = "illumos", target_os = "solaris"))] - use libc::{B153600, B307200, B460800, B921600}; - - match s { - B0 => Ok(BaudRate::B0), - B50 => Ok(BaudRate::B50), - B75 => Ok(BaudRate::B75), - B110 => Ok(BaudRate::B110), - B134 => Ok(BaudRate::B134), - B150 => Ok(BaudRate::B150), - B200 => Ok(BaudRate::B200), - B300 => Ok(BaudRate::B300), - B600 => Ok(BaudRate::B600), - B1200 => Ok(BaudRate::B1200), - B1800 => Ok(BaudRate::B1800), - B2400 => Ok(BaudRate::B2400), - B4800 => Ok(BaudRate::B4800), - #[cfg(any(target_os = "dragonfly", - target_os = "freebsd", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd"))] - B7200 => Ok(BaudRate::B7200), - B9600 => Ok(BaudRate::B9600), - #[cfg(any(target_os = "dragonfly", - target_os = "freebsd", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd"))] - B14400 => Ok(BaudRate::B14400), - B19200 => Ok(BaudRate::B19200), - #[cfg(any(target_os = "dragonfly", - target_os = "freebsd", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd"))] - B28800 => Ok(BaudRate::B28800), - B38400 => Ok(BaudRate::B38400), - B57600 => Ok(BaudRate::B57600), - #[cfg(any(target_os = "dragonfly", - target_os = "freebsd", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd"))] - B76800 => Ok(BaudRate::B76800), - B115200 => Ok(BaudRate::B115200), - #[cfg(any(target_os = "illumos", - target_os = "solaris"))] - B153600 => Ok(BaudRate::B153600), - B230400 => Ok(BaudRate::B230400), - #[cfg(any(target_os = "illumos", - target_os = "solaris"))] - B307200 => Ok(BaudRate::B307200), - #[cfg(any(target_os = "android", - target_os = "freebsd", - target_os = "illumos", - target_os = "linux", - target_os = "netbsd", - target_os = "solaris"))] - B460800 => Ok(BaudRate::B460800), - #[cfg(any(target_os = "android", target_os = "linux"))] - B500000 => Ok(BaudRate::B500000), - #[cfg(any(target_os = "android", target_os = "linux"))] - B576000 => Ok(BaudRate::B576000), - #[cfg(any(target_os = "android", - target_os = "freebsd", - target_os = "illumos", - target_os = "linux", - target_os = "netbsd", - target_os = "solaris"))] - B921600 => Ok(BaudRate::B921600), - #[cfg(any(target_os = "android", target_os = "linux"))] - B1000000 => Ok(BaudRate::B1000000), - #[cfg(any(target_os = "android", target_os = "linux"))] - B1152000 => Ok(BaudRate::B1152000), - #[cfg(any(target_os = "android", target_os = "linux"))] - B1500000 => Ok(BaudRate::B1500000), - #[cfg(any(target_os = "android", target_os = "linux"))] - B2000000 => Ok(BaudRate::B2000000), - #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] - B2500000 => Ok(BaudRate::B2500000), - #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] - B3000000 => Ok(BaudRate::B3000000), - #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] - B3500000 => Ok(BaudRate::B3500000), - #[cfg(any(target_os = "android", all(target_os = "linux", not(target_arch = "sparc64"))))] - B4000000 => Ok(BaudRate::B4000000), - _ => Err(Error::from(Errno::EINVAL)) - } - } + impl TryFrom } #[cfg(any(target_os = "freebsd", @@ -1118,6 +1006,7 @@ pub fn tcgetsid(fd: RawFd) -> Result { #[cfg(test)] mod test { use super::*; + use std::convert::TryFrom; #[test] fn try_from() {