From 96dfa97473450d6e4cbd93b0ffa8a11021096298 Mon Sep 17 00:00:00 2001
From: Nick Mathewson <nickm@torproject.org>
Date: Fri, 10 Jun 2022 10:07:45 -0400
Subject: [PATCH] API-fix for extend_sample_as_needed.

Previously, the API said "you need to call this in a loop till it
returns false".  We did that in one place, but not another.

With the introduction of filters, forgetting to loop here becomes a
bug: so instead, change the behavior of extend_sample_as_needed so
it handles looping itself.
---
 crates/tor-guardmgr/src/lib.rs    | 13 +++----------
 crates/tor-guardmgr/src/sample.rs | 26 +++++++++++++++++---------
 2 files changed, 20 insertions(+), 19 deletions(-)

diff --git a/crates/tor-guardmgr/src/lib.rs b/crates/tor-guardmgr/src/lib.rs
index 7e662efd2..50257f0ac 100644
--- a/crates/tor-guardmgr/src/lib.rs
+++ b/crates/tor-guardmgr/src/lib.rs
@@ -766,16 +766,9 @@ impl GuardMgrInner {
             self.guards
                 .active_guards_mut()
                 .update_status_from_netdir(netdir);
-            loop {
-                let added_any = self.guards.active_guards_mut().extend_sample_as_needed(
-                    now,
-                    &self.params,
-                    netdir,
-                );
-                if !added_any {
-                    break;
-                }
-            }
+            self.guards
+                .active_guards_mut()
+                .extend_sample_as_needed(now, &self.params, netdir);
         }
 
         self.guards
diff --git a/crates/tor-guardmgr/src/sample.rs b/crates/tor-guardmgr/src/sample.rs
index c889febbc..b7ddf0da8 100644
--- a/crates/tor-guardmgr/src/sample.rs
+++ b/crates/tor-guardmgr/src/sample.rs
@@ -274,21 +274,29 @@ impl GuardSet {
     /// Guards always start out un-confirmed.
     ///
     /// Return true if any guards were added.
-    ///
-    /// # Complications
-    ///
-    /// For spec conformance, we only consider our filter when
-    /// selecting new guards if the filter is "very restrictive".
-    /// That makes it possible that this will add fewer
-    /// filter-permitted guards than we had wanted.  Because of that,
-    /// it's advisable to run this function in a loop until it returns
-    /// false.
     pub(crate) fn extend_sample_as_needed(
         &mut self,
         now: SystemTime,
         params: &GuardParams,
         dir: &NetDir,
     ) -> bool {
+        let mut any_added = false;
+        while self.extend_sample_inner(now, params, dir) {
+            any_added = true;
+        }
+        any_added
+    }
+
+    /// Implementation helper for extend_sample_as_needed.
+    ///
+    /// # Complications
+    ///
+    /// For spec conformance, we only consider our filter when selecting new
+    /// guards if the filter is "very restrictive". That makes it possible that
+    /// this function will add fewer filter-permitted guards than we had wanted.
+    /// Because of that, this is a separate function, and
+    /// extend_sample_as_needed runs it in a loop until it returns false.
+    fn extend_sample_inner(&mut self, now: SystemTime, params: &GuardParams, dir: &NetDir) -> bool {
         self.assert_consistency();
         let n_filtered_usable = self
             .guards