Return an iterator, because that's how it's used

Author: mkeeter

common/src/impacted_blocks.rs

@@ -2,7 +2,7 @@
 
 use super::*;
 
-use std::{iter::FusedIterator, ops::RangeInclusive};
+use std::iter::FusedIterator;
 
 #[derive(Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
 pub struct ImpactedAddr {
@@ -137,12 +137,11 @@ impl ImpactedBlocks {
         self == &ImpactedBlocks::Empty
     }
 
-    /// Return a range of impacted extents
+    /// Return an iterator over impacted extents
     pub fn extents(
         &self,
         ddef: &RegionDefinition,
-    ) -> Option<RangeInclusive<u32>> {
-        // XXX return a range of `ExtentId` once `std::iter::Step` is stable
+    ) -> impl Iterator<Item = ExtentId> {
         let blocks_per_extent = ddef.extent_size().value;
         match self {
             ImpactedBlocks::Empty => None, /* empty range */
@@ -151,6 +150,9 @@ impl ImpactedBlocks {
                     ..=((lst.0 / blocks_per_extent) as u32),
             ),
         }
+        .into_iter()
+        .flatten()
+        .map(ExtentId)
     }
 
     pub fn blocks(&self) -> ImpactedBlockIter {

upstairs/src/downstairs.rs

@@ -2081,12 +2081,7 @@ impl Downstairs {
         };
         let ddef = self.ddef.as_ref().unwrap();
         let mut future_repair = false;
-        for eid in impacted_blocks
-            .extents(ddef)
-            .into_iter()
-            .flatten()
-            .map(ExtentId)
-        {
+        for eid in impacted_blocks.extents(ddef) {
             if eid == *eur.start() {
                 future_repair = true;
             } else if eid > *eur.start() && (eid <= *eur.end() || future_repair)
@@ -3511,17 +3506,14 @@ impl Downstairs {
         self.ds_active.get_blocks_for(ds_id)
     }
 
-    /// Return the extent range covered by the given job
+    /// Returns a list of extents covered by the given job
     ///
     /// # Panics
     /// If the job is not stored in our `Downstairs`
     #[cfg(test)]
-    pub fn get_extents_for(
-        &self,
-        ds_id: JobId,
-    ) -> std::ops::RangeInclusive<u32> {
+    pub fn get_extents_for(&self, ds_id: JobId) -> Vec<ExtentId> {
         let ddef = self.ddef.as_ref().unwrap();
-        self.get_blocks_for(ds_id).extents(ddef).unwrap()
+        self.get_blocks_for(ds_id).extents(ddef).collect()
     }
 
     #[cfg(test)]

upstairs/src/upstairs.rs

@@ -3375,9 +3375,9 @@ pub(crate) mod test {
         assert_eq!(jobs.len(), 3);
 
         // confirm which extents are impacted (in case make_upstairs changes)
-        assert_eq!(ds.get_extents_for(jobs[0]).count(), 1);
-        assert_eq!(ds.get_extents_for(jobs[1]).count(), 2);
-        assert_eq!(ds.get_extents_for(jobs[2]).count(), 1);
+        assert_eq!(ds.get_extents_for(jobs[0]).len(), 1);
+        assert_eq!(ds.get_extents_for(jobs[1]).len(), 2);
+        assert_eq!(ds.get_extents_for(jobs[2]).len(), 1);
         assert_ne!(ds.get_extents_for(jobs[0]), ds.get_extents_for(jobs[2]));
 
         // confirm deps
@@ -3438,11 +3438,11 @@ pub(crate) mod test {
         assert_eq!(jobs.len(), 5);
 
         // confirm which extents are impacted (in case make_upstairs changes)
-        assert_eq!(ds.get_extents_for(jobs[0]).count(), 1);
-        assert_eq!(ds.get_extents_for(jobs[1]).count(), 2);
-        assert_eq!(ds.get_extents_for(jobs[2]).count(), 1);
-        assert_eq!(ds.get_extents_for(jobs[3]).count(), 2);
-        assert_eq!(ds.get_extents_for(jobs[4]).count(), 1);
+        assert_eq!(ds.get_extents_for(jobs[0]).len(), 1);
+        assert_eq!(ds.get_extents_for(jobs[1]).len(), 2);
+        assert_eq!(ds.get_extents_for(jobs[2]).len(), 1);
+        assert_eq!(ds.get_extents_for(jobs[3]).len(), 2);
+        assert_eq!(ds.get_extents_for(jobs[4]).len(), 1);
 
         assert_ne!(ds.get_extents_for(jobs[0]), ds.get_extents_for(jobs[2]));
         assert_ne!(ds.get_extents_for(jobs[4]), ds.get_extents_for(jobs[2]));
@@ -3494,9 +3494,9 @@ pub(crate) mod test {
         assert_eq!(jobs.len(), 3);
 
         // confirm which extents are impacted (in case make_upstairs changes)
-        assert_eq!(ds.get_extents_for(jobs[0]).count(), 1);
-        assert_eq!(ds.get_extents_for(jobs[1]).count(), 1);
-        assert_eq!(ds.get_extents_for(jobs[2]).count(), 2);
+        assert_eq!(ds.get_extents_for(jobs[0]).len(), 1);
+        assert_eq!(ds.get_extents_for(jobs[1]).len(), 1);
+        assert_eq!(ds.get_extents_for(jobs[2]).len(), 2);
 
         assert_ne!(ds.get_extents_for(jobs[0]), ds.get_extents_for(jobs[1]));