[PATCH] D113030: Add a new tool for parallel safe bisection, "llvm-bisectd".

[Saleem Abdulrasool via Phabricator via llvm-commits]

compnerd requested changes to this revision.
compnerd added inline comments.
This revision now requires changes to proceed.


================
Comment at: llvm/lib/Support/RemoteBisectorClient.cpp:23
+#include <unistd.h>
+#endif // LLVM_ON_UNIX
+
----------------
This doesn't cover the non-Unix path, where you need to add an include for `WinSock2.h`, `Windows.h`, and possibly `WinSock.h`.


================
Comment at: llvm/lib/Support/RemoteBisectorClient.cpp:37
+  hints.ai_flags = AI_PASSIVE;
+  int Stat = ::getaddrinfo(NULL, std::to_string(PortNumber).c_str(), &hints,
+                           &servinfo);
----------------
Can you please introduce some wrappers for all the BSD socket functions?  On Windows, `GetAddrInfoW` would be preferable over `getaddrinfo`.  Additionally, where do you initialize the sockets library?  (Yes, that is not a thing on Linux/macOS, but on Windows, before you can use any socket function, you need to invoke `WSAStartup`.  This needs a proper hook point.


================
Comment at: llvm/tools/llvm-bisectd/CMakeLists.txt:20
+  target_link_libraries(llvm-bisectd PRIVATE socket)
+endif()
----------------
Please add a case for WIndows, where you need to link against `Ws2_32`.


================
Comment at: llvm/tools/llvm-bisectd/llvm-bisectd.cpp:119
+    // Linger on close.
+    struct linger Linger;
+    Linger.l_onoff = 1;
----------------
I don't know if there is a `struct linger` available on WIndows, it may need to be spelt `LINGER`.


================
Comment at: llvm/tools/llvm-bisectd/llvm-bisectd.cpp:187
+  hints.ai_flags = AI_PASSIVE;
+  int Stat = ::getaddrinfo(NULL, ListenPort.c_str(), &hints, &servinfo);
+  if (Stat) {
----------------
`GetAddrInfoW` should be preferred over `getaddrinfo` on Windows IIRC


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113030/new/

https://reviews.llvm.org/D113030