Skip to content

Fix macOS local network access - #45

Closed
max90T wants to merge 1 commit into
Return-To-The-Roots:masterfrom
max90T:master
Closed

max90T wants to merge 1 commit into
Return-To-The-Roots:masterfrom
max90T:master

Conversation

@max90T

@max90T max90T commented Oct 29, 2026

Copy link
Copy Markdown

LAN game hosting was broken on macOS, making it impossible to host local multiplayer games.

PeerAddr::GetSize() returned the size of the entire sockaddr_storage structure instead of the IPv4 address structure (sockaddr_in). This caused sendto() to fail on macOS with EINVAL.

The fix uses sizeof(addr.sa_in) to return the correct address length. A related Info.plist change is required in the main s25client repository for the complete fix.

@Flamefire

Copy link
Copy Markdown
Member

Which call exactly was failing? I think we need to handle the IPv4 vs IPv6 somewhere because in general I think this approach is the intended one: Use sockaddr_storage if you don't care about sockaddr_in vs sockaddr_in6 and let the OS handle it.
So not sure if passing the v4 size is correct and we wouldn't introduce the same problem just for v6 with this change

@max90T

max90T commented Oct 29, 2026

Copy link
Copy Markdown
Author

Unfortunately, I'm not very familiar with the networking internals, my agent found the issue and fixed it minimally. I tested it and it works on my Mac.

Here is its answer:

Which call exactly was failing

Socket::Send(const void* const buffer, const int length, const PeerAddr& addr) in
libs/network/src/Socket.cpp:709:

return sendto(socket_, reinterpret_cast<const char*>(buffer), length, 0, addr.GetAddr(), addr.GetSize());

PeerAddr::GetSize() (libs/network/include/s25util/Socket.h:81) returns sizeof(addr),
where addr is the address_t union, i.e. sizeof(sockaddr_storage) = 128.
That value is passed as the addrlen argument of sendto().

Why it fails

On Darwin sendto() validates addrlen against the address family and requires the
exact length. Anything else returns EINVAL. Measured on macOS 27.2 (arm64) with a
plain C repro (socket(AF_INET/AF_INET6, SOCK_DGRAM) + sendto):

sizeof(sockaddr)=16 sockaddr_in=16 sockaddr_in6=28 sockaddr_storage=128
IPv4 udp 127.0.0.1:3666 addrlen=16         -> 8 bytes sent
IPv4 udp 127.0.0.1:3666 addrlen=17         -> -1 errno=22 (Invalid argument)
IPv4 udp 127.0.0.1:3666 addrlen=32         -> -1 errno=22 (Invalid argument)
IPv4 udp 127.0.0.1:3666 addrlen=128        -> -1 errno=22 (Invalid argument)
IPv4 bcast 255.255.255.255 addrlen=16      -> 8 bytes sent
IPv4 bcast 255.255.255.255 addrlen=128     -> -1 errno=22 (Invalid argument)
IPv6 udp ::1:3666 addrlen=28               -> 8 bytes sent
IPv6 udp ::1:3666 addrlen=29               -> -1 errno=22 (Invalid argument)
IPv6 udp ::1:3666 addrlen=128              -> -1 errno=22 (Invalid argument)

The 17 and 29 cases are included to show that this is not a "too large" limit but an
exact-match requirement.

Linux only checks that addrlen fits into sockaddr_storage, which is why this never
showed up there.

Effect

PeerAddr is used by the LAN discovery only (LANDiscoveryService::Broadcast for the
server announce, LANDiscoveryClient::Refresh for the query), both over an AF_INET
socket. So on macOS no discovery packet is ever sent: the Mac can neither announce a
game nor find one.

About passing the IPv4 size (your IPv6 remark)

You are right that sockaddr_storage is the correct container type but the wrong
length argument. And you are also right that hardcoding the v4 size alone would just
move the problem to v6 — note that addrlen = 128 fails for AF_INET6 as well (the
correct value there is 28).

With the current code, though, the one-line change cannot regress v6: PeerAddr has a
single constructor (AF_INET + INADDR_BROADCAST) and is only used by the LAN discovery
on an AF_INET socket. So 16 is correct for every call site that exists today.

Two possible ways to go:

Option A – minimal (what my fork currently has)

-    int GetSize() const { return sizeof(addr); }
+    int GetSize() const { return sizeof(addr.sa_in); }

Option B – family-aware, if you want it to be v6-correct as well

// Socket.h, class PeerAddr
int GetSize() const
{
    return addr.sa.sa_family == AF_INET6 ? static_cast<int>(sizeof(addr.sa_in6))
                                         : static_cast<int>(sizeof(addr.sa_in));
}
// Socket.cpp, Socket::Recv(void* const buffer, const int length, PeerAddr& addr)
// in/out parameter: buffer capacity, not the address length
socklen_t addrLen = sizeof(address_t);

Option B needs the Recv line as well: addr is default-constructed there (the header
explicitly says "Uninitilized value!"), so a family-dependent GetSize() would read
uninitialized memory in that path.

GetSize() is currently used in exactly two places with two different meanings:

Call site Line Meaning Correct value
Socket::Recv(..., PeerAddr&) 683 buffer capacity (in/out) sizeof(address_t) = 128
Socket::Send(..., const PeerAddr&) 709 address length exact: 16 (v4) / 28 (v6)

@Flow86

Flow86 commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

Option B would be the correct option then. the GetSize must be protocol-aware, if sendto is not

@Flow86

Flow86 commented Oct 29, 2026 •

Copy link
Copy Markdown
Member

See #46 - Can you test it?

@Flow86 Flow86 closed this Oct 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants