> On March 26, 2016, 8:05 p.m., Daniel Pravat wrote: > > 3rdparty/libprocess/include/process/network.hpp, line 75 > > <https://reviews.apache.org/r/45314/diff/1/?file=1314163#file1314163line75> > > > > We tried to avoid axecuting too much code between ::connect > > WSAGetLastError() calls. > > Constructing the parameter for WindowsSocketError may overwrite the > > last error. It should be better to pass in the return from > > WSAGetLastError() as a parameter to the constructor, that is evaluate > > before the std::string parameter.
Gave this some thought. I'm inclined to keep this as is. Even if you're suggesting that we provide `::WSAGetLastError()` on every callsite like: ``` int result = ::connect(...); // BE AWARE! // Even ignoring `::WSAGetLastError()` being Windows-specific. return ConnectError(::WSAGetLastError(), "Failed to connect to " + stringify(address)); ``` The order of evaluation of the parameters is unspecified. Even if it were to be specified (left-to-right), we still need to be mindful of the code between `::connect` and `ConnectError`. We would run into the same sequence of code execution if someone were to pull out the expression and constructed a variable. ``` int result = ::connect(...); // BE AWARE! std::string message = "Failed to connect to " + stringify(address); // BE AWARE! return ConnectError(::WSAGetLastError(), message); ``` The only way we would completely solve this is if we were to say either: `::WSAGetLastError()` must be called immediately after `::connect`, or ensure that nothing between `::connect` and the call to `::WSAGetLastError()` can overwrite the error code. The latter is what we currently do, and I don't think that reordering just the `::WSAGetLastError` call and the argument to `ConnectError` is a big win. Please share your ideas and solutions! - Michael ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/45314/#review125549 ----------------------------------------------------------- On March 24, 2016, 8:23 p.m., Michael Park wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/45314/ > ----------------------------------------------------------- > > (Updated March 24, 2016, 8:23 p.m.) > > > Review request for mesos, Alex Clemmer and Joris Van Remoortere. > > > Repository: mesos > > > Description > ------- > > See summary. > > > Diffs > ----- > > 3rdparty/libprocess/include/process/network.hpp > 9976257d2d13316062bc95a22ab564ca0df165e5 > 3rdparty/libprocess/src/poll_socket.cpp > 6e6634b4b352e3723096521843546cf56ec6dd8b > > Diff: https://reviews.apache.org/r/45314/diff/ > > > Testing > ------- > > `make check` > > > Thanks, > > Michael Park > >
