[MLton] INetSock problems, hton/ntoh usage

[Matthew Fluet]

Matthew Fluet wrote:
> Vesa Karvonen wrote:
>> The port number of a socket is a 16-bit integer (the sin_port field of
>> struct sockaddr_in).  OTOH, the current net code in MLton represents port
>> numbers as C_Int.t values which usually aren't 16-bit integers.
> 
> I think the better solution would be to use Word16.word for ports in the 
> INetSock structure (and C-side functions in INetSock.c).  It probably 
> isn't worth creating a whole C_InPort structure, when we know it should 
> be equivalent to Word16.  My general approach has been to follow the C 
> types as closely as possible, since it makes the C-side code very simple.
> 
> The Basis Library doesn't specify the behavior of INetSock.toAddr (ia, 
> i) when the port i (of ML type int) is too large to be represented by 
> the C-side representation of a port.  I think we currently would raise 
> the Overflow exception, which seems reasonable.

I propose the following patch.  (This raises SysError with inval rather 
than Overflow when the port cannot be converted from Int.int to 
Word16.word without truncation.)  This works with regression/socket.sml 
and regression/echo.sml, as well as the testcase posed by
   http://mlton.org/pipermail/mlton/2006-December/029465.html

Vesa, does this work with your smlbot?

-------------- next part --------------
Index: runtime/basis-ffi.h
===================================================================
--- runtime/basis-ffi.h	(revision 5441)
+++ runtime/basis-ffi.h	(working copy)
@@ -927,8 +927,8 @@
 extern const C_Int_t Socket_INetSock_Ctl_TCP_NODELAY;
 void Socket_INetSock_fromAddr(Vector(Word8_t));
 void Socket_INetSock_getInAddr(Array(Word8_t));
-C_Int_t Socket_INetSock_getPort(void);
-void Socket_INetSock_toAddr(Vector(Word8_t),C_Int_t,Array(Word8_t),Ref(C_Socklen_t));
+Word16_t Socket_INetSock_getPort(void);
+void Socket_INetSock_toAddr(Vector(Word8_t),Word16_t,Array(Word8_t),Ref(C_Socklen_t));
 C_Errno_t(C_Int_t) Socket_listen(C_Sock_t,C_Int_t);
 extern const C_Int_t Socket_MSG_CTRUNC;
 extern const C_Int_t Socket_MSG_DONTROUTE;
Index: runtime/basis/Net/Socket/INetSock.c
===================================================================
--- runtime/basis/Net/Socket/INetSock.c	(revision 5441)
+++ runtime/basis/Net/Socket/INetSock.c	(working copy)
@@ -1,17 +1,17 @@
 #include "platform.h"
 
 void 
-Socket_INetSock_toAddr (Vector(Word8_t) in_addr, C_Int_t port, 
+Socket_INetSock_toAddr (Vector(Word8_t) in_addr, Word16_t port, 
                         Array(Word8_t) addr, Ref(C_Socklen_t) addrlen) {
   struct sockaddr_in *sa = (struct sockaddr_in*)addr;
 
   sa->sin_family = AF_INET;
-  sa->sin_port = port;
+  sa->sin_port = (uint16_t)port;
   sa->sin_addr = *(const struct in_addr*)in_addr;
   *((socklen_t*)addrlen) = sizeof(struct sockaddr_in);
 }
 
-static int fromAddr_port;
+static uint16_t fromAddr_port;
 static struct in_addr fromAddr_in_addr;
 
 void Socket_INetSock_fromAddr (Vector(Word8_t) addr) {
@@ -22,8 +22,8 @@
   fromAddr_in_addr = sa->sin_addr;
 }
 
-C_Int_t Socket_INetSock_getPort (void) {
-  return fromAddr_port;
+Word16_t Socket_INetSock_getPort (void) {
+  return (Word16_t)fromAddr_port;
 }
 
 void Socket_INetSock_getInAddr (Array(Word8_t) addr) {
Index: runtime/gen/basis-ffi.def
===================================================================
--- runtime/gen/basis-ffi.def	(revision 5441)
+++ runtime/gen/basis-ffi.def	(working copy)
@@ -810,8 +810,8 @@
 Socket.INetSock.Ctl.TCP_NODELAY = _const : C_Int.t
 Socket.INetSock.fromAddr = _import : Word8.t vector -> unit
 Socket.INetSock.getInAddr = _import : Word8.t array -> unit
-Socket.INetSock.getPort = _import : unit -> C_Int.t
-Socket.INetSock.toAddr = _import : Word8.t vector * C_Int.t * Word8.t array * C_Socklen.t ref -> unit
+Socket.INetSock.getPort = _import : unit -> Word16.t
+Socket.INetSock.toAddr = _import : Word8.t vector * Word16.t * Word8.t array * C_Socklen.t ref -> unit
 Socket.MSG_CTRUNC = _const : C_Int.t
 Socket.MSG_DONTROUTE = _const : C_Int.t
 Socket.MSG_DONTWAIT = _const : C_Int.t
Index: runtime/gen/basis-ffi.h
===================================================================
--- runtime/gen/basis-ffi.h	(revision 5441)
+++ runtime/gen/basis-ffi.h	(working copy)
@@ -927,8 +927,8 @@
 extern const C_Int_t Socket_INetSock_Ctl_TCP_NODELAY;
 void Socket_INetSock_fromAddr(Vector(Word8_t));
 void Socket_INetSock_getInAddr(Array(Word8_t));
-C_Int_t Socket_INetSock_getPort(void);
-void Socket_INetSock_toAddr(Vector(Word8_t),C_Int_t,Array(Word8_t),Ref(C_Socklen_t));
+Word16_t Socket_INetSock_getPort(void);
+void Socket_INetSock_toAddr(Vector(Word8_t),Word16_t,Array(Word8_t),Ref(C_Socklen_t));
 C_Errno_t(C_Int_t) Socket_listen(C_Sock_t,C_Int_t);
 extern const C_Int_t Socket_MSG_CTRUNC;
 extern const C_Int_t Socket_MSG_DONTROUTE;
Index: runtime/gen/basis-ffi.sml
===================================================================
--- runtime/gen/basis-ffi.sml	(revision 5441)
+++ runtime/gen/basis-ffi.sml	(working copy)
@@ -1132,8 +1132,8 @@
 end
 val fromAddr = _import "Socket_INetSock_fromAddr" : (Word8.t) vector -> unit;
 val getInAddr = _import "Socket_INetSock_getInAddr" : (Word8.t) array -> unit;
-val getPort = _import "Socket_INetSock_getPort" : unit -> C_Int.t;
-val toAddr = _import "Socket_INetSock_toAddr" : (Word8.t) vector * C_Int.t * (Word8.t) array * (C_Socklen.t) ref -> unit;
+val getPort = _import "Socket_INetSock_getPort" : unit -> Word16.t;
+val toAddr = _import "Socket_INetSock_toAddr" : (Word8.t) vector * Word16.t * (Word8.t) array * (C_Socklen.t) ref -> unit;
 end
 val listen = _import "Socket_listen" : C_Sock.t * C_Int.t -> (C_Int.t) C_Errno.t;
 val MSG_CTRUNC = _const "Socket_MSG_CTRUNC" : C_Int.t;
Index: basis-library/net/inet-sock.sml
===================================================================
--- basis-library/net/inet-sock.sml	(revision 5441)
+++ basis-library/net/inet-sock.sml	(working copy)
@@ -19,18 +19,15 @@
 
       fun toAddr (in_addr, port) =
          let
-            val port = C_Int.fromInt port
-            val port = Net.C_Int.hton port
+            val port = Word16.fromInt port
+                       handle Overflow => PosixError.raiseSys PosixError.inval
+            val port = Net.Word16.hton port
+            val (sa, salen, finish) = Socket.new_sock_addr ()
+            val _ = Prim.toAddr (NetHostDB.inAddrToWord8Vector in_addr,
+                                 port, sa, salen)
+
          in
-            if C_Int.< (port, 0) orelse C_Int.>= (port, 0x10000)
-               then PosixError.raiseSys PosixError.inval
-               else let
-                       val (sa, salen, finish) = Socket.new_sock_addr ()
-                       val _ = Prim.toAddr (NetHostDB.inAddrToWord8Vector in_addr,
-                                            port, sa, salen)
-                    in
-                       finish ()
-                    end
+            finish ()
          end
 
       fun any port = toAddr (NetHostDB.any (), port)
@@ -39,8 +36,8 @@
         let
           val () = Prim.fromAddr (Socket.unpackSockAddr sa)
           val port = Prim.getPort ()
-          val port = Net.C_Int.ntoh port
-          val port = C_Int.toInt port
+          val port = Net.Word16.ntoh port
+          val port = Word16.toInt port
           val (ia, finish) = NetHostDB.new_in_addr ()
           val _ = Prim.getInAddr (NetHostDB.preInAddrToWord8Array ia)
         in
Index: basis-library/net/net.sig
===================================================================
--- basis-library/net/net.sig	(revision 5441)
+++ basis-library/net/net.sig	(working copy)
@@ -7,6 +7,11 @@
 
 signature NET =
    sig
+      structure Word16 :
+         sig
+            val hton: Word16.word -> Word16.word
+            val ntoh: Word16.word -> Word16.word
+         end
       structure C_Int :
          sig
             val hton: C_Int.t -> C_Int.t
Index: basis-library/primitive/basis-ffi.sml
===================================================================
--- basis-library/primitive/basis-ffi.sml	(revision 5441)
+++ basis-library/primitive/basis-ffi.sml	(working copy)
@@ -1132,8 +1132,8 @@
 end
 val fromAddr = _import "Socket_INetSock_fromAddr" : (Word8.t) vector -> unit;
 val getInAddr = _import "Socket_INetSock_getInAddr" : (Word8.t) array -> unit;
-val getPort = _import "Socket_INetSock_getPort" : unit -> C_Int.t;
-val toAddr = _import "Socket_INetSock_toAddr" : (Word8.t) vector * C_Int.t * (Word8.t) array * (C_Socklen.t) ref -> unit;
+val getPort = _import "Socket_INetSock_getPort" : unit -> Word16.t;
+val toAddr = _import "Socket_INetSock_toAddr" : (Word8.t) vector * Word16.t * (Word8.t) array * (C_Socklen.t) ref -> unit;
 end
 val listen = _import "Socket_listen" : C_Sock.t * C_Int.t -> (C_Int.t) C_Errno.t;
 val MSG_CTRUNC = _const "Socket_MSG_CTRUNC" : C_Int.t;