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.

Sign up for GitHub

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 epoll_create1. #384

Closed
wants to merge 5 commits into from
Closed

Add epoll_create1. #384

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4b4223b
Add epoll_create1 and set EPOLL_CLOEXEC flag by default.
kubo39 Jul 9, 2016
5246ae6
Not to set EPOLL_CLOEXEC by default.
kubo39 Jul 9, 2016
b570db9
Update libc version.
kubo39 Jul 15, 2016
390c037
Use git master as libc version.
kubo39 Jul 22, 2016
1945f34
Use libc's declarations
kubo39 Jul 23, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/sys/epoll.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,19 @@ mod ffi {

extern {
pub fn epoll_create(size: c_int) -> c_int;
pub fn epoll_create1(flags: c_int) -> c_int;
pub fn epoll_ctl(epfd: c_int, op: c_int, fd: c_int, event: *const EpollEvent) -> c_int;
pub fn epoll_wait(epfd: c_int, events: *mut EpollEvent, max_events: c_int, timeout: c_int) -> c_int;
Copy link
Member

@posborne posborne Jul 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @kubo39 , although this is not directly related to your PR we have been moving to defining ffi constants entirely in libc and using those rather than defining our own (see CONVENTIONS.md). it looks like the existing constants are present but epoll_create1 is not. Can you please do the following:

  1. Open up a PR to libc adding epoll_create1
  2. Update epoll.rs to use the ffi definitions from libc.

Also, thanks for the change!

}
}

bitflags!(
flags EpollFdFlag: c_int {
const EPOLL_NONBLOCK = 0x800,
const EPOLL_CLOEXEC = 0x80000
Copy link
Member

Choose a reason for hiding this comment

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

These constants should also come out of libc if possible.

}
);

bitflags!(
#[repr(C)]
flags EpollEventKind: u32 {
Expand Down Expand Up @@ -79,6 +87,13 @@ pub fn epoll_create() -> Result<RawFd> {
Errno::result(res)
}

#[inline]
pub fn epoll_create1(flags: EpollFdFlag) -> Result<RawFd> {
let res = unsafe { ffi::epoll_create1(flags.bits() | EPOLL_CLOEXEC.bits) };
Copy link
Contributor

@alex-gulyas alex-gulyas Jul 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

  • The way you are setting the default flag is actually enforcing it, therefore making it impossible for someone to call epoll_create1 without the EPOLL_CLOEXEC flag.
  • I don't think that any of the flags should be turned on by default, especially not without documentation. (Principle of Least Surprise)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In Rust's standard library close-on-exec flag is set by default.
I'm not sure why so, however, I think it's security reason.
Thugh, someone might be surpised when I force to using fcntl(fd, F_SETFD, !O_CLOEXEC) if he want.
Okay, I'll fix it.


Errno::result(res)
}

#[inline]
pub fn epoll_ctl(epfd: RawFd, op: EpollOp, fd: RawFd, event: &EpollEvent) -> Result<()> {
let res = unsafe { ffi::epoll_ctl(epfd, op as c_int, fd, event as *const EpollEvent) };
Expand Down