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.

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

Mark schedule_with_runloop/unschedule_from_runloop as unsafe #47

Merged
merged 3 commits into from
Jan 31, 2024

Conversation

faern
Copy link
Member

@faern faern commented Jan 30, 2024

This is an API breaking change. Mark SCNetworkReachability::schedule_with_runloop and unschedule_from_runloop as unsafe. They accept a raw pointer that it dereferences (this library does not, but it's passed on to Apple libraries that might).

Figuring out a safe API around this is left as an exercise for the future or external contributors.


This change is Reviewable

@faern faern requested a review from dlon January 30, 2024 15:04
Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

Reviewed 2 of 2 files at r1, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


system-configuration/src/network_reachability.rs line 199 at r1 (raw file):

        run_loop_mode: CFStringRef,
    ) -> Result<(), SchedulingError> {
        if unsafe {

Nit: Pointless unsafe

@faern faern force-pushed the mark-runloop-scheduling-as-unsafe branch from c2c7fa2 to 4fcc05c Compare January 30, 2024 15:49
Copy link
Member Author

@faern faern left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 1 of 2 files reviewed, all discussions resolved (waiting on @dlon)


system-configuration/src/network_reachability.rs line 199 at r1 (raw file):

Previously, dlon (David Lönnhager) wrote…

Nit: Pointless unsafe

Soon it won't be 😅 Luckily we change this madness in Rust 2024 to require unsafe blocks in unsafe functions as it should have always been ;) rust-lang/rfcs#2585

But the CI is complaining, so I guess we have to remove it for now. Add it back when switching to Rust 2024

Copy link
Member

@dlon dlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved


system-configuration/src/network_reachability.rs line 199 at r1 (raw file):

Previously, faern (Linus Färnstrand) wrote…

Soon it won't be 😅 Luckily we change this madness in Rust 2024 to require unsafe blocks in unsafe functions as it should have always been ;) rust-lang/rfcs#2585

But the CI is complaining, so I guess we have to remove it for now. Add it back when switching to Rust 2024

TIL. That seems very reasonable.

@faern faern merged commit fe56d19 into main Jan 31, 2024
9 checks passed
@faern faern deleted the mark-runloop-scheduling-as-unsafe branch January 31, 2024 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants