Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Mark schedule_with_runloop/unschedule_from_runloop as unsafe #47

Merged
merged 3 commits into from
Jan 31, 2024
Merged

Mark schedule_with_runloop/unschedule_from_runloop as unsafe #47

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8370ad2
Mark schedule_with_runloop/unschedule_from_runloop as unsafe
faern Jan 30, 2024
4fcc05c
Wrap unsafe calls in unsafe {}
faern Jan 30, 2024
5a08503
Remove superfluous unsafe {}
faern Jan 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`.
Expand Down
103 changes: 58 additions & 45 deletions system-configuration/src/network_reachability.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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();
}
}
}

Expand All @@ -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();
}
}
}

Expand All @@ -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!(
Expand All @@ -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();
Expand Down
Loading