I expect this to fail on our Windows CIs, but it is running now, so we will see in a moment :)

I expect this to fail on our Windows CIs, but it is running now, so we will see in a moment :)

Oh, I wasn't aware that the windows code also ran that code path. Doesn't winsock define its own error codes though? I don't understand how EWOULDBLOCK would work on windows in that case.

We cheat:
https://github.com/OpenTTD/OpenTTD/blob/master/src/network/core/os_abstraction.h#L33

:)

Bit of cross-posting, but I asked in the upstream ticket if serenity only fires EAGAIN and never EWOULDBLOCK, as that would allow different solutions.

Historical, these 2 could be triggered for different circumstances, but in reality the OSes (and libcs) we support don't do this anymore for decades now. So it makes me wonder if we should change all our if statements, or just guard against them being different in os_abstraction.h, and in case of Serenity, map EWOULDBLOCK to EAGAIN. But that does depend on knowing that they didn't implement the old-way-of-doing-stuff and that both can be fired from recv() and co :) As that would mean the solution this PR presents is the only way to go :)

The main reason I was triggered by this, as because for Windows there is no need to check 2 values, so it all becomes a bit derpy.

The other solution would be to make it a function in os_abstraction.h that specifically returns if the socket was blocking, meaning each OS can pick their flavour for this.

I might also be overthinking this completely :P

EDIT: upstream indicates they only fire EAGAIN, never EWOULDBLOCK.

Oh, so it's #define'd to be cross-platform. I'm very surprised that the Windows CI task passed then, maybe the Windows' provided winsock header also cheats the same way? :)

Serenity will probably end up defining both of them to the same number anyway (to simplify other ports), so this PR wouldn't really be needed on our end, but I figure it might be worth merging anyway, since it's a POSIX thing and other operating systems may assume it works this way. We could do a similar trick as the EWOULDBLOCK windows hack to make sure the windows version works, just add #define EAGAIN WSAEWOULDBLOCK as well.

Yeah, Windows is annoying. It defines EWOULDBLOCK and EAGAIN, which works fine with errno. But .. sockets do not use errno. Instead, they use WSAGetLastError and WSAWOULDBLOCK. They give 0 guarantees that EWOULDBLOCK and WSAWOULDBLOCK map to the same value. So yeah .. we need some lovely glue.

As alternative to your PR, maybe something like this works too:
master...TrueBrain:egain-vs-ewouldblock

Follows the portability advise of POSIX, without us having to worry about it ever again. Not sure it is a better solution, but while I fight that out with other devs, would you mind testing if that in fact also does solve the issue? :D

Tnx!

I pulled your branch, it works on serenity. I think that is a better way to solve it, I'll leave this PR open for now until you guys have figured out which approach you like. :)