DescentDevelopers / Descent3

Descent 3 by Outrage Entertainment
GNU General Public License v3.0
2.74k stars 231 forks source link

Replace ioctl with fcntl, fix ioctlsocket calls (64-bit fix) #356

Closed GravisZro closed 1 month ago

GravisZro commented 2 months ago

These take two different types of arguments but worked in the past due to long being 32-bit.

Pull Request Type

Description

The ioctl argument for UNIX systems is int* while the ioctlsocket argument for Windows is unsigned long*. On 32-bit systems theses coincided so no problems but not on 64-bit. However, the behavior of ioctl calls is non-standard which is why code should use fnctl.

See also:

Checklist

Additional Comments

winterheart commented 1 month ago

I would rewrite socket initialization as

#ifdef WIN32

dedicated_listen_socket = socket(AF_INET, SOCK_STREAM, 0);

// ...

unsigned long arg = 1;
ioctlsocket(dedicated_listen_socket, FIONBIO, &arg);

#else // WIN32

#ifndef SOCK_NONBLOCK
#include <fcntl.h>
#define SOCK_NONBLOCK O_NONBLOCK
#endif

dedicated_listen_socket = socket(AF_INET, SOCK_STREAM | SOCK_NONBLOCK, 0);

#endif

And you don't have to use additional fcntl(..., F_SETFL (fcntl(..., F_GETFL) )) calls on Linux / macOS platforms. There a plenty code duplication around socket initialization, so would be some helper functions would be useful.

GravisZro commented 1 month ago

@winterheart Not all of these are socket initializations. As such, I consolidated the code into a single function in networking.h. Including networking.h in other headers lead to conflicting duplicate code which is why there are now code deletions.

GravisZro commented 1 month ago

Would someone please merge this? It's been reviewed by two people already.

winterheart commented 1 month ago

PR has regression on Linux, see #398.