Merge: fd/time: add tests, docs, cleanups

Merge pull request #405 from aparcar/fd-tests-cleanup

rosenpass/src/protocol/protocol.rs

@@ -91,19 +91,13 @@ use rosenpass_ciphers::kem::{EphemeralKem, StaticKem};
 use rosenpass_ciphers::{aead, xaead, KEY_LEN};
 use rosenpass_constant_time as constant_time;
 use rosenpass_secret_memory::{Public, PublicBox, Secret};
-use rosenpass_util::{cat, mem::cpy_min, ord::max_usize, time::Timebase};
+use rosenpass_util::{cat, mem::cpy_min, time::Timebase};
 use zerocopy::{AsBytes, FromBytes, Ref};
 
 use crate::{hash_domains, msgs::*, RosenpassError};
 
 // CONSTANTS & SETTINGS //////////////////////////
 
-/// Size required to fit any message in binary form
-pub const RTX_BUFFER_SIZE: usize = max_usize(
-    size_of::<Envelope<InitHello>>(),
-    size_of::<Envelope<InitConf>>(),
-);
-
 /// A type for time, e.g. for backoff before re-tries
 pub type Timing = f64;
 

util/src/controlflow.rs

@@ -1,4 +1,7 @@
+/// A collection of control flow utility macros
+
 #[macro_export]
+/// A simple for loop to repeat a $body a number of times
 macro_rules! repeat {
     ($times:expr, $body:expr) => {
         for _ in 0..($times) {
@@ -8,6 +11,7 @@ macro_rules! repeat {
 }
 
 #[macro_export]
+/// Return unless the condition $cond is true, with return value $val, if given.
 macro_rules! return_unless {
     ($cond:expr) => {
         if !($cond) {
@@ -22,6 +26,7 @@ macro_rules! return_unless {
 }
 
 #[macro_export]
+/// Return if the condition $cond is true, with return value $val, if given.
 macro_rules! return_if {
     ($cond:expr) => {
         if $cond {
@@ -36,13 +41,14 @@ macro_rules! return_if {
 }
 
 #[macro_export]
+/// Break unless the condition is true, from the loop with label $val, if given.
 macro_rules! break_if {
     ($cond:expr) => {
         if $cond {
             break;
         }
     };
-    ($cond:expr, $val:expr) => {
+    ($cond:expr, $val:tt) => {
         if $cond {
             break $val;
         }
@@ -50,15 +56,94 @@ macro_rules! break_if {
 }
 
 #[macro_export]
+/// Continue if the condition is true, in the loop with label $val, if given.
 macro_rules! continue_if {
     ($cond:expr) => {
         if $cond {
             continue;
         }
     };
-    ($cond:expr, $val:expr) => {
+    ($cond:expr, $val:tt) => {
         if $cond {
-            break $val;
+            continue $val;
         }
     };
 }
+
+#[cfg(test)]
+mod tests {
+    #[test]
+    fn test_repeat() {
+        let mut sum = 0;
+        repeat!(10, {
+            sum += 1;
+        });
+        assert_eq!(sum, 10);
+    }
+
+    #[test]
+    fn test_return_unless() {
+        fn test_fn() -> i32 {
+            return_unless!(true, 1);
+            0
+        }
+        assert_eq!(test_fn(), 0);
+
+        fn test_fn2() -> i32 {
+            return_unless!(false, 1);
+            0
+        }
+        assert_eq!(test_fn2(), 1);
+    }
+
+    #[test]
+    fn test_return_if() {
+        fn test_fn() -> i32 {
+            return_if!(true, 1);
+            0
+        }
+        assert_eq!(test_fn(), 1);
+
+        fn test_fn2() -> i32 {
+            return_if!(false, 1);
+            0
+        }
+        assert_eq!(test_fn2(), 0);
+    }
+
+    #[test]
+    fn test_break_if() {
+        let mut sum = 0;
+        for i in 0..10 {
+            break_if!(i == 5);
+            sum += 1;
+        }
+        assert_eq!(sum, 5);
+
+        let mut sum = 0;
+        'one: for _ in 0..10 {
+            for j in 0..20 {
+                break_if!(j == 5, 'one);
+                sum += 1;
+            }
+        }
+        assert_eq!(sum, 5);
+    }
+
+    #[test]
+    fn test_continue_if() {
+        let mut sum = 0;
+        for i in 0..10 {
+            continue_if!(i == 5);
+            sum += 1;
+        }
+        assert_eq!(sum, 9);
+
+        let mut sum = 0;
+        'one: for i in 0..10 {
+            continue_if!(i == 5, 'one);
+            sum += 1;
+        }
+        assert_eq!(sum, 9);
+    }
+}

util/src/fd.rs

@@ -13,11 +13,6 @@ use crate::{mem::Forgetting, result::OkExt};
 /// The old file descriptor is masked to avoid potential use after free (on file descriptor)
 /// in case the given file descriptor is still used somewhere
 pub fn claim_fd(fd: RawFd) -> rustix::io::Result<OwnedFd> {
-    // check if valid fd
-    if !(0..=i32::MAX).contains(&fd) {
-        return Err(rustix::io::Errno::BADF);
-    }
-
     let new = clone_fd_cloexec(unsafe { BorrowedFd::borrow_raw(fd) })?;
     mask_fd(fd)?;
     Ok(new)
@@ -268,25 +263,17 @@ mod tests {
     }
 
     #[test]
-    fn test_claim_fd_invalid() {
+    #[should_panic(expected = "fd != u32::MAX as RawFd")]
+    fn test_claim_fd_invalid_neg() {
         let fd: RawFd = -1;
-        let result = claim_fd(fd);
-        assert!(result.is_err());
-        assert_eq!(
-            result.unwrap_err().to_string(),
-            "Bad file descriptor (os error 9)"
-        );
+        let _ = claim_fd(fd);
     }
 
     #[test]
+    #[should_panic(expected = "fd != u32::MAX as RawFd")]
     fn test_claim_fd_invalid_max() {
         let fd: RawFd = i64::MAX as RawFd;
-        let result = claim_fd(fd);
-        assert!(result.is_err());
-        assert_eq!(
-            result.unwrap_err().to_string(),
-            "Bad file descriptor (os error 9)"
-        );
+        let _ = claim_fd(fd);
     }
 
     #[test]

util/src/lib.rs

@@ -11,7 +11,6 @@ pub mod length_prefix_encoding;
 pub mod mem;
 pub mod mio;
 pub mod option;
-pub mod ord;
 pub mod result;
 pub mod time;
 pub mod typenum;

util/src/ord.rs

@@ -1,8 +0,0 @@
-// TODO remove this once std::cmp::max becomes const
-pub const fn max_usize(a: usize, b: usize) -> usize {
-    if a > b {
-        a
-    } else {
-        b
-    }
-}

util/src/time.rs

@@ -1,20 +1,53 @@
-use std::time::{Duration, Instant};
+use std::time::Instant;
+
+/// A timebase.
+///
+/// This is a simple wrapper around `std::time::Instant` that provides a
+/// convenient way to get the seconds elapsed since the creation of the
+/// `Timebase` instance.
 
 #[derive(Clone, Debug)]
 pub struct Timebase(Instant);
 
 impl Default for Timebase {
+    // TODO: Implement new()?
     fn default() -> Self {
         Self(Instant::now())
     }
 }
 
 impl Timebase {
+    /// Returns the seconds elapsed since the creation of the `Timebase`
     pub fn now(&self) -> f64 {
         self.0.elapsed().as_secs_f64()
     }
+}
 
-    pub fn dur(&self, t: f64) -> Duration {
-        Duration::from_secs_f64(t)
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use std::thread::sleep;
+    use std::time::Duration;
+
+    #[test]
+    fn test_timebase() {
+        let timebase = Timebase::default();
+        let now = timebase.now();
+        assert!(now > 0.0);
+    }
+
+    #[test]
+    fn test_timebase_clone() {
+        let timebase = Timebase::default();
+        let timebase_clone = timebase.clone();
+        assert_eq!(timebase.0, timebase_clone.0);
+    }
+
+    #[test]
+    fn test_timebase_sleep() {
+        let timebase = Timebase::default();
+        sleep(Duration::from_secs(1));
+        let now = timebase.now();
+        assert!(now > 1.0);
     }
 }