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

Don't set the nofile rlimit if the max is infinity #106

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

xSetech
Copy link

@xSetech xSetech commented Jul 7, 2019

  • Subtraction occurred on a negative-integer (enum for infinity) resulting in
    an EINVAL from setrlimit().

  • On macOS, setting the limit to RLIM_INFINITY is an error (see man setrlimit).

@coveralls
Copy link

coveralls commented Jul 7, 2019

Coverage Status

Coverage decreased (-0.06%) to 74.212% when pulling 851a05e on xSetech:nofile into 37bfc5a on worr:master.

library/src/client.c Outdated Show resolved Hide resolved
- Subtraction occurred on a negative-integer (enum for infinity) resulting in
  an EINVAL from setrlimit().

- On macOS, setting the limit to RLIM_INFINITY is an error (see `man
  setrlimit`).
@worr
Copy link
Owner

worr commented Jul 12, 2019

If you force push, please leave a comment, since github doesn't give me notifications for those.

That said, while the code looks good, we raise the nofile for a reason - specifically because we're file descriptor hungry, what with having all those simultaneous ssh connections. It's probably better to detect infinity, and set the soft limit to that rather than to short-change the user on file descriptors.

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