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

proposal: x/sys/unix: support for all fields of EpollEvent #32192

Closed
wI2L opened this issue May 22, 2019 · 3 comments
Closed

proposal: x/sys/unix: support for all fields of EpollEvent #32192

wI2L opened this issue May 22, 2019 · 3 comments

Comments

@wI2L
Copy link
Contributor

wI2L commented May 22, 2019

Similar to this discussion about kqueue, and related to this closed issue, it would be nice to add the missing fields of the C type epoll_event defined as the following to the equivalent unix.EpollEvent type in Go.

     typedef union epoll_data {
           void    *ptr;
           int      fd;
           uint32_t u32;
           uint64_t u64;
       } epoll_data_t;

       struct epoll_event {
           uint32_t     events;    /* Epoll events */
           epoll_data_t data;      /* User data variable */
       };

/cc @tklauser @bradfitz @ianlancetaylor

@gopherbot gopherbot added this to the Proposal milestone May 22, 2019
@bradfitz
Copy link
Contributor

EpollEvent is currently https://godoc.org/golang.org/x/sys/unix#EpollEvent :

type EpollEvent struct {
    Events uint32
    Fd     int32
    Pad    int32
}

Go can't do unions, though. Even with unsafe, it's invalid to put a non-pointer in an unsafe.Pointer or a pointer in an integer. Both can lead to crashes.

To support both uint64 and unsafe.Pointer data values, we'd need two EpollEvent structs (e.g. EpollEventInt and EpollEventPtr) and then two EpollCtl variants.

But that doesn't help with EpollWait which can return a mix of events. (some with ints, some with pointers)

But perhaps we could just make the rule be that a given fd can only have one or the other and then you choose the epoll_wait variant that you used with epoll_ctl. shrug

@wI2L
Copy link
Contributor Author

wI2L commented May 22, 2019

Thanks for the concise answer. Ian mentioned the lack of union in Go in the issue I cross-referenced, but it just clicked in my head after reading you and re-reading the epoll man page.

Following up what you said, could we instead choosing one of the two cases (the pointer one) as an opiniated solution instead ? One could always store the pointer to a int64 var if necessary, but the inverse isn't true.

As an example, my current use case for a pointer field would be to store a pointer to a net.Conn registered with the epoll instance (understand it's fd). This avoids having to manage a separate map that references each Conn by it's fd (which is the only useful "identifier" for an event pulled from epoll_wait).

@rsc
Copy link
Contributor

rsc commented May 28, 2019

Merging into #32153.

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

No branches or pull requests

4 participants