Skip to content

Allow specifying resource in posix_getrlimit() for single result #9790

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

Closed

Conversation

iluuu1994
Copy link
Member

Fetching all resource limits seems incredibly wasteful, and the API also becomes more awkward (posix_getrlimit()['soft core']). This PR allows specifying a resource, returning [$softLimit, $hardLimit] that can be destructured.

@iluuu1994 iluuu1994 force-pushed the posix_getrlimit-single-resource branch from 5fbe6e9 to f1a6592 Compare October 21, 2022 11:57
@iluuu1994 iluuu1994 requested a review from cmb69 October 21, 2022 15:48
Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

This looks like a useful feature to me. I'm not quite sure, though, about the "unlimited" return value; while that matches what we have so far, yielding null might be more reasonable (after all, there is no limit).

@iluuu1994
Copy link
Member Author

@cmb69 I've thought about this as well and thought it might be better to stay consistent in terms of values returned. Returning two distinct values for unlimited within the same function might be a bit of a WTF. If we do use a different value it might make more sense to return RLIM_INFINITY and expose that as a userland constant (since that can't be interpreted as a valid resource limit by the set function).

If we really want to do that, I think it might be better to introduce a new function altogether.

@iluuu1994
Copy link
Member Author

iluuu1994 commented Oct 22, 2022

Any ideas for a new function name? posix_get_single_rlimit? I couldn't come up with anything sensible, since the good name is already taken 😛 (it also really should be called getrlimits)

@cmb69
Copy link
Member

cmb69 commented Oct 22, 2022

Any ideas for a new function name?

I don't think a new function is necessary (my reaction was because you're reasoning to just return "unlimited"). IMO, this is good as is. But if we want to bikeshed, we should do this on internals. :)

@iluuu1994
Copy link
Member Author

@cmb69 Do you think this needs a mail to the internals?

Copy link
Member

@cmb69 cmb69 left a comment

Choose a reason for hiding this comment

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

Do you think this needs a mail to the internals?

No. But it needs an entry in UPGRADING. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants