Page MenuHomePhabricator

[lldb-server] Introduce Socket::Initialize and Terminate to simply WSASocket setup
ClosedPublic

Authored by asmith on Apr 8 2019, 11:37 PM.

Diff Detail

Event Timeline

asmith created this revision.Apr 8 2019, 11:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptApr 8 2019, 11:37 PM
labath accepted this revision.Apr 9 2019, 12:15 AM

Thank you for doing this. We probably should have done something like this a long time ago. I have a couple of inline comments, but they're all very trivial. Feel free to commit, if you agree with them.

source/Host/common/Socket.cpp
95–96

I'd make this llvm::errorCodeToError(llvm::mapWindowsError(::WSAGetLastError())); as it's somewhat shorter and hopefully produces a better error message.

103

we don't use (void) in functions taking no arguments.

source/Initialization/SystemInitializerCommon.cpp
123–126

These calls should be roughly in dependency order. Since Sockets are in Host, I'd probably put this somewhere around the HostInfo::Initialize call on line 93. (same goes for the terminate call below)

unittests/Host/MainLoopTest.cpp
23

ASSERT_THAT_ERROR(Socket::Initialize(), llvm::Succeeded()). That way, if this ever fails, you'll get the actual error message instead of "false is not true". (you might need to include something to have these available).

This revision is now accepted and ready to land.Apr 9 2019, 12:15 AM
asmith updated this revision to Diff 194443.Apr 9 2019, 9:53 PM

Addressed the review comments

asmith marked 3 inline comments as done.Apr 9 2019, 9:53 PM
asmith marked an inline comment as done.
asmith closed this revision.Apr 9 2019, 9:55 PM