LibCore: Fix SocketAddress.h compilation errors on Windows #2673
Conversation
stasoid
force-pushed
the
sockaddr
branch
5 times, most recently
from
8506667 to
fca2d78
Compare
stasoid
force-pushed
the
sockaddr
branch
4 times, most recently
from
e6fa91b to
6ece3e4
Compare
|
@R-Goc Does it work for you? |
|
It's creating a bit of an issue in system.h and systemWindows.cpp |
|
I addressed this issue. You can include winsock2.h after sockaddr-win.h. |
|
Ok I'll check. Maybe clang format is moving includes around. |
|
While clang-fromat was indeed moving includes around, turns out the issue is with ws2tcpip.h. Looks like with sockaddr-win.h included the only thing needed from there is gai_strerror(). Though that can be replaced with WSAGetLastError, which would mean different error handling for windows and linux, but I think that's fine. That makes it a bit ugly as getting an error string from windows error messages is a bit difficult, but for now I'll return an int. |
|
Should we just change AF_UNIX in SocketAddress.h to AF_LOCAL? Or define AF_UNIX to AF_LOCAL here? |
|
Yes, sockaddr-win.h doesn't work with ws2tcpip.h. If a declaration from ws2tcpip.h is needed it is supposed to be included in sockaddr-win.h. However, I don't want to include gai_strerror because it is inline and Error::from_windows_error can be used instead. AF_UNIX is not used anywhere in the codebase, that's why I didn't define it. |
|
It is in SocketAddress.h. sockaddr-win isn't a replacement for SocketAddress.h |
|
As mentioned on another PR, please make any and all added header files follow the project conventions for naming |
Libraries/LibCore/sockaddr-win.h
Outdated
| extern "C" __stdcall void freeaddrinfo(PADDRINFOA pAddrInfo); | ||
| extern "C" __stdcall PCSTR inet_ntop(int Family, void const* pAddr, char* pStringBuf, size_t StringBufSize); | ||
|
|
||
| #define _WS2DEF_ // don't include ws2def.h |
There was a problem hiding this comment.
Is this sort of header shenanigans common for portable/win32 C++ projects? This particular line makes me a bit uncomfortable. If the declarations here are incompatible with the ones in ws2def.h, then we have a problem.
There was a problem hiding this comment.
They are not incompatible with ws2def.h, they are the same as in ws2def.h and C++ doesn't allow that. This problem exists for two reasons: 1. SocketAddress.h requires definitions of those structs and 2. including winsock2.h in headers is prohibited. Other projects don't have this problem because they allow including windows.h in headers.
|
@ADKaster I renamed the file. |
Including winsock2.h in a header file is undesirable for architectural reasons, so I pulled the needed types from ws2def.h and ws2ipdef.h into a separate header.
Files inaddr.h and afunix.h are tiny and can be included directly. They each define a single struct (
in_addrandsockaddr_unrespectively).I removed the
#if (_WIN32_WINNT < 0x0600)code because it is not relevant for us. It is intended for Windows XP/Windows Server 2003 and earlier, but Ladybird doesn't support 32-bit, and 64-bit Windows XP is very rare.