reduced to 2 classes: PassiveSocket and SimpleSocket .. and some more fixes/additions

[hayguen]

see source!,
should be easier to use:
PassiveSocket for Server things
and SimpleSocket for everything else

[lethosor]

Does this maintain source compatibility (i.e. is it possible to still use CActiveSocket)?

[hayguen]

hmmm, you are right, my patch breaks compatibility!
that's why i had to fix/update the examples.
Cause Accept() delivered CActiveSocket*, a child of CSimpleSocket.
I think changing CActiveSocket to a typedef should fix that ...

[hayguen]

in the examples it fixed that problem.
have a look yourself!

[hayguen]

my last commit with new examples and many new aliases (inline functions) and a few bugfixes were merged into this pull request ..

[warmist]

Have you tried building dfhack with it?

[hayguen]

Nope. I'm using clsocket for extio_rtl_tcp
https://github.com/hayguen/extio_rtl_tcp
and probably for other projects (in future).

There shouldn't be errors building DFHack .. are there?

[warmist]

oh my bad. Thought that this is not used anywhere else.

[hayguen]

Had found clsockets few years ago. Think, it was at the authors official site, which now is down
It was one of the few or even the only lib with following criteria:

• Windows and Linux
• use in commercial software
• small / easy to use - without full blown framework
• easy static linking

Had used multicast UDP sockets at that time ...
Now using it again, for TCP reception in the extio_rtl_tcp DLL on Windows.

You see any problem using DFHack/clsocket as "base" for further developments?

[warmist]

Nope. Just thought that something more popular exists and we're just using this mostly because it's very simple and we started using it.

[hayguen]

There's nothing wrong, when it's simple!

[lethosor]

Should Tcp6 be here or part of the previous case for Tcp?

[hayguen]

you are definitely right: that code looks wrong. noticed that also.
in fact i did not change the behaviour, just silenced the warnings from gcc, complaining of missing cases.
before fixing those bug(s), we should have a test program, e.g. extend echoserver and txtoserver for ipv6!

[hayguen]

Found out, that there's much more to do, besides the correct switch case, that IPv6 works correctly:
address resolution is broken/deprecated ..

[lethosor]

Do we really need GetNumReceivableBytes()? I had to add checks for _DARWIN in addition to _LINUX to get it to compile, but it's always returning 0. That's probably possible to fix, but I'm still not sure why we need it now if we didn't need it before. The examples only seem to compare it to the number of bytes returned by Receive(), but it doesn't actually affect them.

From here:

Be warned though, the OS doesn't necessarily guarantee how much data it will buffer for you, so if you are waiting for very much data you are going to be better off reading in data as it comes in and storing it in your own buffer until you have everything you need to process something.

And here:

You simply cannot assume that every send() from one end of the connection will result in exactly one recv() on the far end with exactly the same number of bytes.

Basically, if you care about receiving the same number of bytes that were sent, you should probably include the message length in the message, instead of relying on the network to transmit the entire message at once.

I'll still see if I can fix it on OS X if it is needed, but I'm not sure about it.

This is the patch I made to get it to compile, by the way:

diff --git a/src/SimpleSocket.cpp b/src/SimpleSocket.cpp
index 2d0eedd..4c613f9 100644
--- a/src/SimpleSocket.cpp+++ b/src/SimpleSocket.cpp@@ -42,7 +42,7 @@
*----------------------------------------------------------------------------*/
#include "SimpleSocket.h"
-#ifdef _LINUX+#if defined(_LINUX) || defined(_DARWIN)
#include <sys/ioctl.h>
#endif
#include <string.h>
@@ -1326,7 +1326,7 @@ int32 CSimpleSocket::GetNumReceivableBytes()
return -1;
return (int32)numBytesInSocket;
-#elif defined(_LINUX)+#elif defined(_LINUX) || defined(_DARWIN)
int32 numBytesInSocket = 0;
if ( ioctl(m_socket, FIONREAD, (char*)&numBytesInSocket) < 0 )
return -1;

[hayguen]

I definitely need it in some other closed-source project, where i replaced the Qt4 Socket stuff.
So it is used and implicitly tested on Windows 32/64 and Linux.
I read the warnings .. but the exact value of available bytes is not necessary.
We could always return a new error for DARWIN, if that's ok?

[lethosor]

Just Receive() doesn't work? If you're receiving into a buffer, is it not possible to just use a buffer large enough for whatever you need?

I think I found a way to get it working on OS X, though, if it's necessary.

[hayguen]

The company specific close source has its own abstraction for network/tcp connections, where Qt4 was utilized in the back end. Now, replaced that Qt4 - without touching the application layer / logic :-)
You see, i can't use Receive() :-(

[lethosor]

Oh, so you're replacing Qt's equivalent of GetNumReceivableBytes? I guess that would make sense.

[hayguen]

More or less: yes.
Qt3 Compatibility Layer, coming with Qt4, Qt3::QSocketDevice was working fine.
In Qt4, QSocket does handle it's own buffers per thread .. breaking many things!
Now the solution is/was to use clsockets for networking and to allow moving to Qt5.

[hayguen]

i've moved and rebased (merged latest changes from here).
can someone have a look at this PR again?
are there any reasons for not acceptig this PR?
some more bugfixes are in the pipe ..

[lethosor]

I'm pretty sure #4 (comment) still needs to be addressed for macOS.
Other than that, I think I was reluctant to merge originally because it's a large set of changes and I didn't want to break DFHack (or other users). In particular, the removal of CActiveSocket changes a lot of things. That's not necessarily a problem, though. The typedef you added should help. I would still like to verify that DFHack works with these changes, and might not have much time to do that until next week or so, but I'll try to get to this (and other changes) as soon as I'm able. There are a couple other outstanding PRs that I (or someone else) should look at too.

[hayguen]

I'm pretty sure #4 (comment) still needs to be addressed for macOS.

yes, you were right. i rebase/amended your patch.
by the way: not having a macOS for testing .. is there a simple way for remote compilation and executing unittests?

I would still like to verify that DFHack works with these changes

of course

might not have much time to do that until next week or so

sure, that's good .. especially after having all this stuff lying around for years

[lethosor]

I've been meaning to set up GitHub Actions in this repo - I was planning on just Linux (to replace Travis CI) but could probably add macOS and Windows too. That should help make sure it at least compiles on those platforms - not sure how comprehensive tests are currently.

[hayguen]

but could probably add macOS and Windows too

sounds great. i've no experience with this github actions kind of stuff ..

not sure how comprehensive tests are currently

(nearly) all test programs are for manual execution .. but we should be able to figure out some automatic tests

[hayguen]

@lethosor :

and might not have much time to do that until next week or so

i'm not at work for the next 2 weeks from next monday .. but we have several more changes/commits, i'd need to merge ..
you might want to wait until i'm back and get them into the github repo, to test everything at once .. or start testing .. and accept what is already online.
however you prefer ..