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

FreeBSD's rfork wrapper proposal #2121

Merged
merged 1 commit into from
Dec 4, 2023
Merged

Conversation

devnexen
Copy link
Contributor

@devnexen devnexen commented Sep 3, 2023

No description provided.

src/unistd.rs Outdated
feature! {
#![feature = "process"]
#[cfg(target_os = "freebsd")]
pub mod rfork_freebsd {
Copy link
Member

Choose a reason for hiding this comment

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

We don't need to put it in a dedicated module, the existing 2 pub mod within unistd.rs are for better naming

rfork(2), IMHO, is good enough so we don't need this

src/unistd.rs Outdated
Comment on lines 3368 to 2920
/// Flags for [`rfork`]
/// subset of flags supported by FreeBSD 12.x and onwards
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
/// Flags for [`rfork`]
/// subset of flags supported by FreeBSD 12.x and onwards
/// Flags for [`rfork`]
///
/// subset of flags supported by FreeBSD 12.x and onwards

We should add a newline here or the they will be rendered into a single paragraph

src/unistd.rs Outdated
/// subset of flags supported by FreeBSD 12.x and onwards
/// with a safe outcome, thus as `RFMEM` can possibly lead to undefined behavior,
/// it is not in the list. And `rfork_thread` is deprecated.
pub struct RforkFlags: libc::c_int {
Copy link
Member

Choose a reason for hiding this comment

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

Seems like there is a RFSIGSHARE flag that is currently missing, though it is also not available in libc

Copy link
Member

Choose a reason for hiding this comment

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

Noticed this PR rust-lang/libc#3457, thanks for doing it!

@SteveLauC
Copy link
Member

SteveLauC commented Nov 29, 2023

Please revert the format changes.

Is it possible to add a test in test_unistd.rs? And, don't forget a CHANGELOG entry.

@devnexen devnexen force-pushed the rfork_fbsd branch 3 times, most recently from 193749b to 1355dd0 Compare November 29, 2023 18:55
}
}

feature! {
Copy link
Member

Choose a reason for hiding this comment

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

If we use a feature! block here, why not put them all in the block?

feature! {
#![feature = "process"]

#[cfg(target_os = "freebsd")]
libc_bitflags! {
}


#[cfg(target_os = "freebsd")]
pub unsafe fn rfork(flags: RforkFlags) -> Result<ForkResult> {
}

}

@SteveLauC SteveLauC added this pull request to the merge queue Dec 4, 2023
Merged via the queue into nix-rust:master with commit 3a20eef Dec 4, 2023
34 checks passed
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