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

Use up-to-date system call to create epoll context #5821

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

Conversation

lovelle
Copy link
Contributor

@lovelle lovelle commented Jan 30, 2019

epoll_create() is a deprecated, older variant of epoll_create1(). Older versions
of epoll_create() used to take a size argument to provide a hint about the
number of file descriptors to be watched. Nowadays the kernel dynamically sizes
the required data structures.

epoll_create() is a deprecated, older variant of epoll_create1(). Older versions
of epoll_create() used to take a size argument to provide a hint about the
number of file descriptors to be watched. Nowadays the kernel dynamically sizes
the required data structures.
@lovelle
Copy link
Contributor Author

lovelle commented Feb 13, 2019

Hi @antirez! please feel free to close this pull at your own will

@antirez
Copy link
Contributor

antirez commented Apr 26, 2019

Hello @lovelle, AFAIK the old systemcall will just discard the argument and call the new implementation, so why bother?

@lovelle
Copy link
Contributor Author

lovelle commented Apr 30, 2019

@antirez Yes, that's correct, but is an implementation detail, I think is better not using a consider deprecated syscall, at least from a consistency standpoint.

Also, as a detail: The newer syscall is able to prevent any forked process to access to that file descriptor with EPOLL_CLOEXEC flag.

Thanks!

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

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.

None yet

3 participants