labath added a reviewer: labath.
labath added inline comments.
================
Comment at: include/lldb/Host/windows/PosixApi.h:79
+#define WNOHANG 1
+#define WUNTRACED 2
+
----------------
I don't see `WUNTRACED` being used anywhere within lldb. Why did you need this?
================
Comment at: include/lldb/Host/windows/PosixApi.h:108-111
+inline pid_t waitpid(pid_t pid, int *status, int options) {
+ // To be implemented.
+ return pid_t(-1);
+}
----------------
What are your plans for this?
I assume you needed this for the fork-server in lldb-platform.cpp. I think the
best way to address that would be to change the code from spawning a separate
process for each connection to having a new thread for each connection. That
should be much more portable and will avoid the need to mock posix apis on
windows. I can help you with that if you run into any problems there.
================
Comment at: source/Host/common/SocketAddress.cpp:268
+#endif
+ printf("SocketAddress::GetAddressInfo - %s\n", error.AsCString());
}
----------------
Writing to stdout like this is very rude. If there isn't a suitable way to
return this info, then I suggest writing this to the log instead.
================
Comment at: tools/lldb-server/SystemInitializerLLGS.cpp:37-52
+#if defined(_WIN32)
+ if (g_ref_count++ == 0) {
+ // Require Windows Sockets version 2.2.
+ auto wVersion = MAKEWORD(2, 2);
+ WSADATA wsaData;
+ auto err = WSAStartup(wVersion, &wsaData);
+ if (err == 0) {
----------------
I think a better option here would be to move this code to something like
`Socket::Initialize`. Then we could call this from
`SystemInitializerCommon::Initialize`, and it would be available to everyone.
This would allow us to remove the copy of this code in PlatformWindows, and
replace the `WSAStartup` calls in various unittests with `Socket::Initialize()`
================
Comment at: tools/lldb-server/lldb-server.cpp:40
-static void initialize() {
+namespace llsvr {
+void initialize() {
----------------
In other places we use `llgs` (for `LL`db `G`db `S`erver), so I suggest
sticking to that.
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56233/new/
https://reviews.llvm.org/D56233
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
