Skip to content

adding pid_type enum for Linux. #4403

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

Merged
merged 1 commit into from
Apr 16, 2025
Merged

Conversation

devnexen
Copy link
Contributor

ref

@rustbot
Copy link
Collaborator

rustbot commented Apr 13, 2025

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

@tgross35
Copy link
Contributor

We should probably avoid adding any more Rust enum s because of the trivial unsoundness - I wrote this up a bit more in #4419. To work around this, I'm adding a c_enum! macro that produces constants rather than a Rust enum #4420. Would you be able to try using that here?

@devnexen
Copy link
Contributor Author

Oh I see yes I ll try it out a bit later, so by quick glancing it, we can t choose the underlying type of the values right ?

@tgross35
Copy link
Contributor

The macro for this should expand to

pub type pid_type = c_uint;
pub const PIDTYPE_PID: pid_type = 0;
pub const PIDTYPE_TGID: pid_type = 1
// etc;

I made it accept #[repr(u16)] to change the type alias if that is needed. I think the default c_uint should be fine here though, right?

@devnexen devnexen force-pushed the prctl_pid_type branch 2 times, most recently from 2b93449 to b7fe45c Compare April 16, 2025 05:12
@@ -92,6 +92,16 @@ e! {
}
}

c_enum! {
e {
Copy link
Contributor

Choose a reason for hiding this comment

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

This identifier is the name of the type alias, so should be pid_type here. I should add some docs...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

of course ... thanks :)

Copy link
Contributor

@tgross35 tgross35 left a comment

Choose a reason for hiding this comment

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

Thanks!

@tgross35 tgross35 enabled auto-merge April 16, 2025 08:55
@tgross35 tgross35 added this pull request to the merge queue Apr 16, 2025
Merged via the queue into rust-lang:main with commit 72f9d58 Apr 16, 2025
52 checks passed
@tgross35 tgross35 added the stable-nominated This PR should be considered for cherry-pick to libc's stable release branch label Apr 16, 2025
@tgross35 tgross35 mentioned this pull request May 28, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
O-linux O-unix S-waiting-on-review stable-nominated This PR should be considered for cherry-pick to libc's stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants