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

Add getrlimit(2) and setrlimit(2) #1190

Closed
wants to merge 15 commits into from
Closed

Conversation

j1ah0ng
Copy link

@j1ah0ng j1ah0ng commented Mar 6, 2020

This is a continuation of #879. I've added comments and removed usage of the now-deprecated std::mem::uninitialized() function.

Copy link
Member

@asomers asomers left a comment

Choose a reason for hiding this comment

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

You also need to fix the segfaults, of course. I'll try to come back and more fully review this at another time,when I'm more rested.

//! Configure the process resource limits.
use std::mem;

#[cfg(not(any(target_os = "freebsd", target_os = "openbsd", target_os = "netbsd", target_os = "macos")))]
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't use a negative config check here. Nix could add a new platform at any time, and it's easier to maintain checks like this if they're all written as positive checks.


/// Get the current processes resource limits
///
/// A value of None indicates that there's no limit.
Copy link
Member

Choose a reason for hiding this comment

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

None should be in backticks

@LMJW LMJW mentioned this pull request Oct 3, 2020
@asomers
Copy link
Member

asomers commented Oct 3, 2020

Superseded by #1302

@asomers asomers closed this Oct 3, 2020
bors bot added a commit that referenced this pull request Aug 1, 2021
1302: Add getrlimit and setrlimit r=asomers a=LMJW

This PR is based on the previous PR #1190, which has not been updated for a very long time.

I have fixed the segfault and compilation error of the original PR and rebased the changes to the latest master branch.

Please let me know if anything need to been changed.

Co-authored-by: LMJW <[email protected]>
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.

3 participants