Repository: fujiapple852/trippy (language: rust)

Issue:
Off-by-one bug in max-rounds
This bug causes the tracer to run for `max_rounds + 1` rounds.

This bug is does not impact reports which terminate after the correct number of rounds and exit the process and thus stopping the `tracer` thread.  

This bug is observed when using the tracer programatically and setting `max_rounds`.


Discussion / hints:

Fix patch:
diff --git a/src/report.rs b/src/report.rs
index 6c6a6252d..1e37f9991 100644
--- a/src/report.rs
+++ b/src/report.rs
@@ -2,8 +2,6 @@ use crate::backend::trace::Trace;
 use anyhow::anyhow;
 use parking_lot::RwLock;
 use std::sync::Arc;
-use std::thread::sleep;
-use std::time::Duration;
 
 pub mod csv;
 pub mod dot;
@@ -24,7 +22,6 @@ fn wait_for_round(trace_data: &Arc<RwLock<Trace>>, report_cycles: usize) -> anyh
         if let Some(err) = trace.error() {
             return Err(anyhow!("error: {}", err));
         }
-        sleep(Duration::from_millis(100));
     }
     Ok(trace)
 }
diff --git a/src/tracing/config.rs b/src/tracing/config.rs
index 2a57a6a65..1c4ebe67f 100644
--- a/src/tracing/config.rs
+++ b/src/tracing/config.rs
@@ -6,6 +6,7 @@ use crate::tracing::types::{
 };
 use std::fmt::{Display, Formatter};
 use std::net::{IpAddr, Ipv4Addr};
+use std::num::NonZeroUsize;
 use std::time::Duration;
 
 pub mod defaults {
@@ -645,11 +646,20 @@ impl Config {
                 "initial_sequence ({initial_sequence}) > {MAX_SEQUENCE}"
             )));
         }
+        let max_rounds = match max_rounds {
+            Some(max_rounds) if max_rounds > 0 => NonZeroUsize::new(max_rounds).map(MaxRounds),
+            Some(_) => {
+                return Err(TracerError::BadConfig(String::from(
+                    "max_rounds must be greater than zero",
+                )));
+            }
+            None => None,
+        };
         Ok(Self {
             target_addr,
             protocol,
             trace_identifier: TraceId(trace_identifier),
-            max_rounds: max_rounds.map(MaxRounds),
+            max_rounds,
             first_ttl: TimeToLive(first_ttl),
             max_ttl: TimeToLive(max_ttl),
             grace_duration,
@@ -686,6 +696,7 @@ impl Default for Config {
 #[cfg(test)]
 mod tests {
     use super::*;
+    use std::num::NonZeroUsize;
 
     const SOURCE_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(1, 1, 1, 1));
     const TARGET_ADDR: IpAddr = IpAddr::V4(Ipv4Addr::new(2, 2, 2, 2));
@@ -750,7 +761,7 @@ mod tests {
     fn test_config_builder_full() {
         let cfg = ConfigBuilder::new(TraceId(0), TARGET_ADDR)
             .protocol(Protocol::Udp)
-            .max_rounds(MaxRounds(10))
+            .max_rounds(MaxRounds(NonZeroUsize::new(10).unwrap()))
             .first_ttl(TimeToLive(2))
             .max_ttl(TimeToLive(16))
             .grace_duration(Duration::from_millis(100))
@@ -764,7 +775,10 @@ mod tests {
         assert_eq!(TraceId(0), cfg.trace_identifier);
         assert_eq!(TARGET_ADDR, cfg.target_addr);
         assert_eq!(Protocol::Udp, cfg.protocol);
-        assert_eq!(Some(MaxRounds(10)), cfg.max_rounds);
+        assert_eq!(
+            Some(MaxRounds(NonZeroUsize::new(10).unwrap())),
+            cfg.max_rounds
+        );
         assert_eq!(TimeToLive(2), cfg.first_ttl);
         assert_eq!(TimeToLive(16), cfg.max_ttl);
         assert_eq!(Duration::from_millis(100), cfg.grace_duration);
diff --git a/src/tracing/tracer.rs b/src/tracing/tracer.rs
index 4679f0f19..1f5bbbb7e 100644
--- a/src/tracing/tracer.rs
+++ b/src/tracing/tracer.rs
@@ -413,7 +413,7 @@ mod state {
         pub fn finished(&self, max_rounds: Option<MaxRounds>) -> bool {
             match max_rounds {
                 None => false,
-                Some(max_rounds) => self.round.0 > max_rounds.0,
+                Some(max_rounds) => self.round.0 > max_rounds.0.get() - 1,
             }
         }
 
diff --git a/src/tracing/types.rs b/src/tracing/types.rs
index 898d95d11..1a1f6b6b5 100644
--- a/src/tracing/types.rs
+++ b/src/tracing/types.rs
@@ -1,12 +1,13 @@
 use derive_more::{Add, AddAssign, Rem, Sub};
+use std::num::NonZeroUsize;
 
 /// `Round` newtype.
 #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Ord, PartialOrd, AddAssign)]
 pub struct Round(pub usize);
 
 /// `MaxRound` newtype.
-#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Ord, PartialOrd)]
-pub struct MaxRounds(pub usize);
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Ord, PartialOrd)]
+pub struct MaxRounds(pub NonZeroUsize);
 
 /// `TimeToLive` (ttl) newtype.
 #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Ord, PartialOrd, Add, Sub, AddAssign)]


Test patch: