Now use epoll so we can wake up readEvents#66
Now use epoll so we can wake up readEvents#66PieterD wants to merge 15 commits intofsnotify:masterfrom
Conversation
|
I like how you separated the poller out into a separate file. But man, this makes it so much more complex. |
inotify_poller.go
Outdated
There was a problem hiding this comment.
I think InotifyInit should remain in NewWatcher and pass the fd into newFdPoller.
There was a problem hiding this comment.
Likewise, NewWatcher should be responsible for closing the inotify fd if poller has an error.
Poller could in theory work for any fd passed in and only clean up its own resources.
|
Should there be inotify_poller_test.go? |
inotify.go
Outdated
There was a problem hiding this comment.
Maybe doneResp instead? Or doneResposne?
I was thinking of sending a signal one way over the done channel and then sending one back, but that won't work if the first signal is to close. Looking at the code more closely, I think we need to keep the two separate channels.
There was a problem hiding this comment.
I'm starting to wonder if we should have a helper function to send an error down the channel or err/abort if done.
inotify_poller.go
Outdated
inotify_poller_test.go
Outdated
There was a problem hiding this comment.
Oh. Mixing named and unamed. Mind you errno isn't really needed here for readability of the API. Could move it to var.
There was a problem hiding this comment.
But it is required to reference the return value in the deferred function two lines below.
fixes #5.
fixes #40.
updates #59 (fixed for inotify).