Merge branch 'mark-runloop-scheduling-as-unsafe'

mullvad · Jan 31, 2024 · fe56d19 · fe56d19
2 parents 7633a12 + 5a08503
commit fe56d19
Show file tree

Hide file tree

Showing 2 changed files with 61 additions and 45 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -24,6 +24,9 @@ Line wrap the file at 100 chars.                                              Th
 ## [Unreleased]
 ### Changed
 - Bump minimum supported Rust version (MSRV) to 1.64.0.
+- Breaking: Mark `SCNetworkReachability::schedule_with_runloop` and `unschedule_from_runloop` as
+  `unsafe`. They accept a raw pointer that it dereferences. Figuring out a safe API around this is
+  left as an exercise for the future.
 
 ### Fixed
 - Fix memory leak in `SCNetworkReachability::set_callback`.

diff --git a/system-configuration/src/network_reachability.rs b/system-configuration/src/network_reachability.rs
@@ -186,18 +186,21 @@ impl SCNetworkReachability {
     /// See [`SCNetworkReachabilityScheduleFromRunLoop`] for details.
     ///
     /// [`SCNetworkReachabilityScheduleFromRunLoop`]: https://developer.apple.com/documentation/systemconfiguration/1514894-scnetworkreachabilityschedulewit?language=objc
-    pub fn schedule_with_runloop(
+    ///
+    /// # Safety
+    ///
+    /// The `run_loop_mode` must not be NULL and must be a pointer to a valid run loop mode.
+    /// Use `core_foundation::runloop::kCFRunLoopCommonModes` if you are unsure.
+    pub unsafe fn schedule_with_runloop(
         &self,
         run_loop: &CFRunLoop,
         run_loop_mode: CFStringRef,
     ) -> Result<(), SchedulingError> {
-        if unsafe {
-            SCNetworkReachabilityScheduleWithRunLoop(
-                self.0,
-                run_loop.to_void() as *mut _,
-                run_loop_mode,
-            )
-        } == 0u8
+        if SCNetworkReachabilityScheduleWithRunLoop(
+            self.0,
+            run_loop.to_void() as *mut _,
+            run_loop_mode,
+        ) == 0u8
         {
             Err(SchedulingError(()))
         } else {
@@ -210,18 +213,21 @@ impl SCNetworkReachability {
     /// See [`SCNetworkReachabilityUnscheduleFromRunLoop`] for details.
     ///
     /// [`SCNetworkReachabilityUnscheduleFromRunLoop`]: https://developer.apple.com/documentation/systemconfiguration/1514899-scnetworkreachabilityunschedulef?language=objc
-    pub fn unschedule_from_runloop(
+    ///
+    /// # Safety
+    ///
+    /// The `run_loop_mode` must not be NULL and must be a pointer to a valid run loop mode.
+    /// Use `core_foundation::runloop::kCFRunLoopCommonModes` if you are unsure.
+    pub unsafe fn unschedule_from_runloop(
         &self,
         run_loop: &CFRunLoop,
         run_loop_mode: CFStringRef,
     ) -> Result<(), UnschedulingError> {
-        if unsafe {
-            SCNetworkReachabilityUnscheduleFromRunLoop(
-                self.0,
-                run_loop.to_void() as *mut _,
-                run_loop_mode,
-            )
-        } == 0u8
+        if SCNetworkReachabilityUnscheduleFromRunLoop(
+            self.0,
+            run_loop.to_void() as *mut _,
+            run_loop_mode,
+        ) == 0u8
         {
             Err(UnschedulingError(()))
         } else {
@@ -383,14 +389,15 @@ mod test {
                 addr
             );
             reachability.set_callback(|_| {}).unwrap();
-            reachability
-                .schedule_with_runloop(&CFRunLoop::get_current(), unsafe { kCFRunLoopCommonModes })
-                .unwrap();
-            reachability
-                .unschedule_from_runloop(&CFRunLoop::get_current(), unsafe {
-                    kCFRunLoopCommonModes
-                })
-                .unwrap();
+            // SAFETY: We use the Apple provided run_loop_mode kCFRunLoopCommonModes
+            unsafe {
+                reachability
+                    .schedule_with_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                    .unwrap();
+                reachability
+                    .unschedule_from_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                    .unwrap();
+            }
         }
     }
 
@@ -414,14 +421,15 @@ mod test {
                 remote
             );
             reachability.set_callback(|_| {}).unwrap();
-            reachability
-                .schedule_with_runloop(&CFRunLoop::get_current(), unsafe { kCFRunLoopCommonModes })
-                .unwrap();
-            reachability
-                .unschedule_from_runloop(&CFRunLoop::get_current(), unsafe {
-                    kCFRunLoopCommonModes
-                })
-                .unwrap();
+            // SAFETY: We use the Apple provided run_loop_mode kCFRunLoopCommonModes
+            unsafe {
+                reachability
+                    .schedule_with_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                    .unwrap();
+                reachability
+                    .unschedule_from_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                    .unwrap();
+            }
         }
     }
 
@@ -435,16 +443,18 @@ mod test {
             match SCNetworkReachability::from_host(&input) {
                 Some(mut reachability) => {
                     reachability.set_callback(|_| {}).unwrap();
-                    reachability
-                        .schedule_with_runloop(&CFRunLoop::get_current(), unsafe {
-                            kCFRunLoopCommonModes
-                        })
-                        .unwrap();
-                    reachability
-                        .unschedule_from_runloop(&CFRunLoop::get_current(), unsafe {
-                            kCFRunLoopCommonModes
-                        })
-                        .unwrap();
+                    // SAFETY: We use the Apple provided run_loop_mode kCFRunLoopCommonModes
+                    unsafe {
+                        reachability
+                            .schedule_with_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                            .unwrap();
+                        reachability
+                            .unschedule_from_runloop(
+                                &CFRunLoop::get_current(),
+                                kCFRunLoopCommonModes,
+                            )
+                            .unwrap();
+                    }
                 }
                 None => {
                     panic!(
@@ -471,9 +481,12 @@ mod test {
             let mut reachability =
                 SCNetworkReachability::from("0.0.0.0:0".parse::<SocketAddr>().unwrap());
             reachability.set_callback(|_| {}).unwrap();
-            reachability
-                .schedule_with_runloop(&CFRunLoop::get_current(), unsafe { kCFRunLoopCommonModes })
-                .unwrap();
+            // SAFETY: We use the Apple provided run_loop_mode kCFRunLoopCommonModes
+            unsafe {
+                reachability
+                    .schedule_with_runloop(&CFRunLoop::get_current(), kCFRunLoopCommonModes)
+                    .unwrap();
+            }
             reachability.set_callback(|_| {}).unwrap();
             let _ = tx.send(reachability);
             CFRunLoop::run_current();