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 nix-rust#373
asomers · Aug 9, 2021 · ec41684 · ec41684
1 parent 68488a4
commit ec41684
Show file tree

Hide file tree

Showing 5 changed files with 153 additions and 144 deletions.
diff --git 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.
+  (#[TODO](https://github.com/nix-rust/nix/pull/TODO))
+
 ### 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.
+  (#[TODO](https://github.com/nix-rust/nix/pull/TODO))
+
 ### 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.
+  (#[TODO](https://github.com/nix-rust/nix/pull/TODO))
 
 ## [0.22.0] - 9 July 2021
 ### Added

diff --git 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,88 +97,171 @@ 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<Self> {
+                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)*
         }
     };
 
     // 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)*
         }
     };
 
     // 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
@@ -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<type_of_event_filter>
 }
 
 #[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<EventFilter> {
+        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::<libc::kevent>(), mem::size_of::<KEvent>());
 }
+
+#[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
@@ -71,6 +71,7 @@ libc_enum!{
                       target_os = "redox")))]
         SIGINFO,
     }
+    impl TryFrom<i32>
 }
 
 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<libc::c_int> for Signal {
-    type Error = Error;
-
-    fn try_from(signum: libc::c_int) -> Result<Signal> {
-        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;