|Anonymous | Login | Signup for a new account||2013-12-06 11:52 CET|
|Main | My View | View Issues | Change Log | Roadmap|
|View Issue Details|
|ID||Project||Category||View Status||Date Submitted||Last Update|
|0005676||OCaml||OCaml windows||public||2012-07-09 19:26||2012-09-25 01:07|
|Target Version||4.01.0+dev||Fixed in Version||4.01.0+dev|
|Summary||0005676: IPv6 support under Windows|
|Description||This patch (against trunk) adds IPv6 support under Windows.|
Windows XP is required for compiling OCaml with IPv6 support. Compiled programs should work with older versions of Windows as well.
There are a lot of include order changes, as winsock2.h needs to be included before winsock.h.
The getnameinfo and getaddrinfo bindings are taken from the unix/ directory.
string_of_inet_addr and inet_addr_of_string are implemented using getnameinfo and getaddrinfo, as functions inet_pton and inet_ntop are only available in very recent versions of Windows.
Finally, the check for PF_INET6 in file socket.c is disabled. (By the way, the comment seems incorrect, as OCaml is already linked against Winsock 2.)
|Tags||No tags attached.|
|Attached Files|| patch.txt [^] (12,422 bytes) 2012-07-09 19:26 [Show Content]
updated-patch.txt [^] (13,057 bytes) 2012-07-11 18:16 [Show Content]
|Thanks! I'll give the patch a try tomorrow and commit on trunk barring any major problems.|
Disclaimer: I'm not a Unix system programming guru, esp. under Windows, so please bear with me, but, I just gave this patch a try and it doesn't seem to work fully.
# open Unix;;
# let a = inet_addr_of_string "2001:6b0:5:1688::200";;
val a : Unix.inet_addr = <abstr>
# let a = ADDR_INET (a, 8001);;
val a : Unix.sockaddr = ADDR_INET (<abstr>, 8001)
# let ic, oc = open_connection a;;
Exception: Unix.Unix_error (EAFNOSUPPORT, "connect", "").
Is that expected?
|I missed this function... I have uploaded an updated patch.|
So I gave your patch a try and everything seems to compile/work fine as far as my light testing could tell.
Here's a few remarks on the patch:
- you sometimes use #if defined() vs. #ifdef, is there a reason?
- you change a lot of includes; apparently, it seems that you want "unixsupport.h" to be now included *before* <windows.h>; is there a reason for that too?
- you sometimes remove the "unixsupport.h" header completely from the includes list, e.g. in "startup.c" -- is that part of a general cleanup?
- why did you add <alloc.h> to times.c?
- there's an extra set_close_on_exec that appeared in open_connection; I'm very much afraid that this will be a surprise for some people...
Apart from that, the patch looks very good!
> - you sometimes use #if defined() vs. #ifdef, is there a reason?
I'm only using "#if defined()" in win32unix/socket.c, and that's
just because I took the code from unix/socket.c...
> - you change a lot of includes; apparently, it seems that you want
> "unixsupport.h" to be now included *before* <windows.h>; is there a reason for
> that too?
"unixsupport.h" used to include <winsock.h>, and I've changed it
to include <winsock2.h> instead. As <winsock2.h> needs to be
included before <windows.h>, I had to change a lot of includes...
An alternative would have been to remove the inclusion of
<winsock.h> from "unixsupport.h" and instead include <winsock2.h>
only when necessary. This seems to involve more changes: there are
more files that involve sockets than files that include <windows.h>.
Another alternative would be to move the inclusion of <windows.h> to
"unixsupport.h". That might be cleaner in the long run (in particular,
we avoid the hack in "winworker.h" mentioned below), but is more
invasive: <windows.h> would be included a lot more often.
> - you sometimes remove the "unixsupport.h" header completely from
> the includes list, e.g. in "startup.c" -- is that part of a general
In "winworker.h", <winsock2.h> needs to be included between
#define _WIN32_WINNT 0x0400
So, I have moved the inclusion of "unixsupport.h" there and
removed it from the files that include "winworker.h".
> - why did you add <alloc.h> to times.c?
There were some missing includes, including this one. Indeed,
this is not IPv6 related...
Here, times.c uses function alloc_small which is declared in <alloc.h>.
> - there's an extra set_close_on_exec that appeared in open_connection;
> I'm very much afraid that this will be a surprise for some people...
A set_close_on_exec was added in unix/unix.ml in revision 6420
(Sat Jun 19 15:33:53 2004 UTC (8 years ago) by xleroy). Indeed,
file descriptors handled through in/out_channel are expected to be
'close_on_exec' by default.
It seems more coherent to have it as well under Windows.
Do you want separate patches for the additional includes (such as
<alloc.h> in times.c) and for the extra set_close_on_exec, or is that
Thanks for the extensive explanations and the bonus cleanups. Everything seems just fine so I just commited this on trunk as r12710
|2012-07-09 19:26||vouillon||New Issue|
|2012-07-09 19:26||vouillon||File Added: patch.txt|
|2012-07-10 16:56||protz||Relationship added||parent of 0005398|
|2012-07-10 16:58||protz||Note Added: 0007691|
|2012-07-10 16:58||protz||Assigned To||=> protz|
|2012-07-10 16:58||protz||Status||new => assigned|
|2012-07-10 16:58||protz||Target Version||=> 4.01.0+dev|
|2012-07-11 17:49||protz||Note Added: 0007718|
|2012-07-11 18:16||vouillon||File Added: updated-patch.txt|
|2012-07-11 18:18||vouillon||Note Added: 0007720|
|2012-07-13 09:40||protz||Note Added: 0007747|
|2012-07-13 11:18||vouillon||Note Added: 0007748|
|2012-07-13 14:16||protz||Note Added: 0007750|
|2012-07-13 14:16||protz||Status||assigned => resolved|
|2012-07-13 14:16||protz||Fixed in Version||=> 4.01.0+dev|
|2012-07-13 14:16||protz||Resolution||open => fixed|
|2012-09-25 01:07||frisch||Relationship added||related to 0005766|
|Copyright © 2000 - 2011 MantisBT Group|