Navigation Menu

Skip to content
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

Consider that IPv6 is always enabled in Windows code #10185

Merged
merged 2 commits into from Feb 10, 2021

Conversation

MisterDA
Copy link
Contributor

@MisterDA MisterDA commented Feb 1, 2021

HAS_IPV6 is hardcoded in the build system, support in Windows has been there for a long time, so we can unconditionally use IPv6.

Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense to assume that Windows supports IPv6 nowadays, I think, but @dra27 knows better.

I would suggest a more defensive change to otherlibs/unix/socket.c (see below), just because I believe in defensive coding for Unix system programming.

otherlibs/unix/socket.c Outdated Show resolved Hide resolved
@MisterDA
Copy link
Contributor Author

MisterDA commented Feb 3, 2021

Done, thank you for your review. David actually warned me that you'd want to keep #elif defined(PF_UNSPEC), but I was too convinced that it had been defined everywhere since a long time ago.

@dra27
Copy link
Member

dra27 commented Feb 3, 2021

Looking at the history, we've actually always been Winsock 2, although it was only when ocamldebug support was merged in 2008 that the "new" ws2_32 library was linked rather than the legacy wsock32.

I believe the APIs have been supported since late 1995... even I don't have a VM that old to hand (at least not one I can start-up since, ahem, Microsoft Virtual PC 2004 was superseded by the way less cool Virtual PC 2007)

otherlibs/unix/socket.c Outdated Show resolved Hide resolved
Copy link
Contributor

@xavierleroy xavierleroy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fine, thanks.

@xavierleroy
Copy link
Contributor

Would you like to add an entry to the Changes file? Your choice.

IPv6 is always enabled on Windows.
@MisterDA
Copy link
Contributor Author

MisterDA commented Feb 9, 2021

I've added a small Changes entry. I wasn't sure if it was needed for that PR. Thank you!

@dra27 dra27 merged commit 3813e7d into ocaml:trunk Feb 10, 2021
@dra27
Copy link
Member

dra27 commented Feb 10, 2021

Thanks!

@MisterDA MisterDA deleted the windows-ipv6 branch April 6, 2023 13:08
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.

None yet

3 participants