Mantis Bug Tracker
Main | My View | View Issues | Change Log | Roadmap
  

View Issue Details [ Jump to Notes ] [ Issue History ] [ Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0005676OCamlOCaml windowspublic2012-07-09 19:262012-09-25 01:07
Reportervouillon 
Assigned Toprotz 
PrioritynormalSeverityfeatureReproducibilityalways
StatusresolvedResolutionfixed 
PlatformOSOS Version
Product Version 
Target Version4.01.0+devFixed in Version4.01.0+dev 
Summary0005676: IPv6 support under Windows
DescriptionThis 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.)
TagsNo tags attached.
Attached Filestxt file icon patch.txt [^] (12,422 bytes) 2012-07-09 19:26 [Show Content]
txt file icon updated-patch.txt [^] (13,057 bytes) 2012-07-11 18:16 [Show Content]

- Relationships
parent of 0005398resolvedprotz Inadequate exception thrown when creating IPv6 socket on win32/msvc 
related to 0005766resolvedfrisch MSVC port broken following switch to winsock2 

-  Notes
(0007691)
protz (manager)
2012-07-10 16:58

 Thanks! I'll give the patch a try tomorrow and commit on trunk barring any major problems.
(0007718)
protz (manager)
2012-07-11 17:49

 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?
(0007720)
vouillon (reporter)
2012-07-11 18:18

 I missed this function... I have uploaded an updated patch.
(0007747)
protz (manager)
2012-07-13 09:40

 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!

Thanks,

jonathan
(0007748)
vouillon (reporter)
2012-07-13 11:18

 > - 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
> cleanup?

In "winworker.h", <winsock2.h> needs to be included between
 #define _WIN32_WINNT 0x0400
and
 #include <windows.h>

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
good enough?
(0007750)
protz (manager)
2012-07-13 14:16

 Thanks for the extensive explanations and the bonus cleanups. Everything seems just fine so I just commited this on trunk as r12710

Cheers,

jonathan

- Issue History
Date Modified Username Field Change
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
Powered by Mantis Bugtracker