Skip to content

Add AF_UNIX support on windows #2115

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 24, 2025
Merged

Conversation

p1-0tr
Copy link
Contributor

@p1-0tr p1-0tr commented Mar 20, 2025

Add support for using unix domain sockets on Windows.

@yhirose
Copy link
Owner

yhirose commented Mar 20, 2025

@p1-0tr could you please add unit tests? Then, I'll start reviewing. Thanks!

@p1-0tr p1-0tr force-pushed the ps-af-unix-windows branch from 1f356d5 to 5cd397a Compare March 21, 2025 08:33
@p1-0tr
Copy link
Contributor Author

p1-0tr commented Mar 21, 2025

@yhirose I enabled the existing test. Only the basic case, because windows only has support for basic features on AF_UNIX (no fd passing, or other anciliary data, autobind, socketpair, etc. - https://devblogs.microsoft.com/commandline/af_unix-comes-to-windows/#unsupported\unavailable: )

@yhirose
Copy link
Owner

yhirose commented Mar 21, 2025

@p1-0tr I just reviewed the code, but the condition looks a bit complicated. Can it be simpler as shown below?

      if (socket_options) { socket_options(sock); }

#ifdef _WIN32
      // Setting SO_REUSEADDR seems not to work well with AF_UNIX on windows, so
      // avoid setting default_socket_options.
      detail::set_socket_opt(sock, SOL_SOCKET, SO_REUSEADDR, 0);
#endif

Signed-off-by: Piotr Stankiewicz <piotr.stankiewicz@docker.com>
@p1-0tr p1-0tr force-pushed the ps-af-unix-windows branch from 5cd397a to 5c8e5da Compare March 24, 2025 07:22
@p1-0tr
Copy link
Contributor Author

p1-0tr commented Mar 24, 2025

@yhirose Yes, that works.

I went with a comparison on the settings callback as I thought it would be more in line with the principle of least surprise (from the user's point of view). But I am happy with the simpler approach :)

@yhirose yhirose merged commit 72b35be into yhirose:master Mar 24, 2025
@yhirose
Copy link
Owner

yhirose commented Mar 24, 2025

@p1-0tr thanks for the contribution!

@yhirose
Copy link
Owner

yhirose commented Mar 25, 2025

@p1-0tr could you take a look at this error on GitHub Actions CI?

https://github.com/yhirose/cpp-httplib/actions/runs/14047884306/job/39343816422

@p1-0tr
Copy link
Contributor Author

p1-0tr commented Mar 25, 2025

@yhirose I created a fix for it - #2118 . Unfortunately ordering of winsock2.h and afunix.h includes matters.

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.

2 participants