From 8370ad27963eaa63fce630ba4ba86c71a6783a21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Tue, 30 Jan 2024 16:02:32 +0100 Subject: [PATCH 1/3] Mark schedule_with_runloop/unschedule_from_runloop as unsafe --- CHANGELOG.md | 3 +++ system-configuration/src/network_reachability.rs | 14 ++++++++++++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b2fd9c0..95bf3a8 100644 --- 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 index 1a1b91a..081a757 100644 --- a/system-configuration/src/network_reachability.rs +++ b/system-configuration/src/network_reachability.rs @@ -186,7 +186,12 @@ 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, @@ -210,7 +215,12 @@ 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, From 4fcc05c387408e4d1af7aad97d8b9a17a124499b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Tue, 30 Jan 2024 16:24:37 +0100 Subject: [PATCH 2/3] Wrap unsafe calls in unsafe {} --- .../src/network_reachability.rs | 65 ++++++++++--------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/system-configuration/src/network_reachability.rs b/system-configuration/src/network_reachability.rs index 081a757..1edb984 100644 --- a/system-configuration/src/network_reachability.rs +++ b/system-configuration/src/network_reachability.rs @@ -393,14 +393,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(); + } } } @@ -424,14 +425,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(); + } } } @@ -445,16 +447,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!( @@ -481,9 +485,12 @@ mod test { let mut reachability = SCNetworkReachability::from("0.0.0.0:0".parse::().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(); From 5a085038789e8fcf636ca59c9c8dfaebb179f2f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Linus=20F=C3=A4rnstrand?= Date: Tue, 30 Jan 2024 17:30:25 +0100 Subject: [PATCH 3/3] Remove superfluous unsafe {} --- .../src/network_reachability.rs | 24 ++++++++----------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/system-configuration/src/network_reachability.rs b/system-configuration/src/network_reachability.rs index 1edb984..2342c5f 100644 --- a/system-configuration/src/network_reachability.rs +++ b/system-configuration/src/network_reachability.rs @@ -196,13 +196,11 @@ impl SCNetworkReachability { 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 { @@ -225,13 +223,11 @@ impl SCNetworkReachability { 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 {