[Lldb-commits] [PATCH] D32600: Resurrect pselect MainLoop implementation

[Pavel Labath via Phabricator via lldb-commits]

labath added inline comments.


================
Comment at: source/Host/common/MainLoop.cpp:82
+  int queue_id;
+  std::vector<struct kevent> events;
+  struct kevent event_list[4];
----------------
eugene wrote:
> here and below struct seems to be redundant
One of them holds the input events, and the other the output -- I am assuming they can't be the same. @beanz?

I am going to rename them to reflect this better though.


================
Comment at: source/Host/common/MainLoop.cpp:135
+
+template <typename F> void MainLoop::RunImpl::ForEachReadFD(F &&f) {
+  assert(num_events >= 0);
----------------
zturner wrote:
> Why do we need this function?  Just have a function that returns an `ArrayRef<int>` and let the caller iterate over it themselves.
The logic for detecting whether an event has actually happened is different for ppoll and kevent based implementations. In case of kevent you already get a list of events ready, so it may seem that it is redundant, but in case of ppoll, I have to look through the list of input events, and check that they have they `POLLIN` flag set. This is basically what is this trying to abstract. I can believe it is overkill though.

I'll see if I can come up with something cleaner.


================
Comment at: source/Host/common/MainLoop.cpp:167
+
+#ifdef FORCE_PSELECT
+  fd_set read_fd_set;
----------------
zturner wrote:
> How about just moving this `#ifdef` up outside of the Poll function?  Given that it occurs so many times, i think it makes the logic and the differences between the two implementations easier to follow.
OK.


https://reviews.llvm.org/D32600