Network API

[xoft]

That is exactly what I expected (WinXP doesn't support the IPV6_V6ONLY socket flag), the important thing now is if the echo server did listen on both IPv6 and IPv4?
Btw, you can do "netstat -na > netstatresults.txt" and then copy&paste the netstatresults.txt file instead of the screenshot It's called output redirection if you want to learn more about it.

[xoft]

I think the current C++ code is ready for a big review. I'd like to ask you to have a look at it, especially all the callbacks used, and say if it's okay. Testing it out would be a good idea, too - I've done tests on Win7 64-bit and on Raspbian. Before building, use this cmake command:

Code:

cmake -DSELF_TEST=1 <srcrootdir>
(or)
cmake -DSELF_TEST=1 -DCMAKE_BUILD_TYPE=Debug <srcrootdir>

This will enable the build of external tests: NameLookup, Google-exe and EchoServer. Each tests one interface of the network API - looking up IP <-> hostname, connecting as a client and downloading data, and opening a server.

@STR how's it going with WinXP, does it now work with both IPv4 and IPv6?

[NiLSPACE]

I can't test it right now due to all my tests. We finish all our tests on Thursday, so I'll do it then if you don't mind

[worktycho]

My feeling about the current API is that cNetwork has too many responsibilities. I feel it should be split up either into three classes, one handling DNS, one handling Connect and one handling Listen, or they should all be made into free functions.

I think that the best outcome would be to split the Network code into three sets of h/cpp files, one holding the two DNS free functions, one holding the connections code and one holding the TCPLink code as most code will need at most two of the files.

[xoft]

If you split the code into multiple files, you will need to expose the internal implementation classes, because they are dependent on each other, as well as the cNetworkSingleton. My goal was to hide the implementation behind a facade that exposes only the minimum interface needed for usage, so that the internals may change dramatically without changing the interface.

[worktycho]

Then maybe we need to put the implementation classes into headers of their own, like with the simulators. My worry is that Network.cpp is only going to grow and grow as people ask for additional features like UDP support. Also it can be difficult to understand the structure at the moment because the code is just a huge lump.

[xoft]

The code can't be split without exposing cNetworkSingleton - most of the implementation relies on its internals. And I don't want to expose it, since it has libevent stuff in it, so we'd need to put libevent dependencies in the header.

I don't think the Network stuff is gonna grow much more. It's pretty much complete on the TCP side, and the UDP side can have a separate file of its own - after all, libevent doesn't have such a high level support for it anyway.

[xoft]

And here I was, thinking that the Network code was actually pretty well written. You seem to be rather opposed to most of it, so how would you write it if you were to write it?

[worktycho]

The code is fine. Its just what files to put it in I disagree about. Once we've sorted that out I reckon its ready to merge.

As for UDP, if UDP goes in a separate file then you need to expose cNetworkSingleton anyway, as UDP will depend on cNetworkSingleton. My feeling is that cNetworkSingleton should be exposed but we just don't use it outside OSSupport. Also if you add a method to access m_DNSBase and m_EventBase then you can get rid of the friend class. Yes it leaks the libevent abstraction but so does accessing them through friends.

[NiLSPACE]

I finished my tests, but when I tried to start it it gave an error on both Windows 8.1 and XP:

Code:

EchoServer: starting up
Failed to create IPv6 MainSock: 10093 (De toepassing heeft WSAStartup niet aangeroepen, of WSAStartup is mislukt.)
Cannot listen on port 9876

The Dutch part means: "The application didn't call WSAStartup, or WSAStartup failed."