[MLton] bug in MLton and bug in basis

Stephen Weeks MLton@mlton.org
Thu, 18 Aug 2005 15:48:37 -0700


> First, the easy bug: in net/socket.sml, MLton uses LSB order even on
> powerpc. This makes the Ctl.* methods return the result from system
> calls with the endian backwards. It has gone unnoticed because most
> of the operations there return/take a bool.

Patch committed.  Thanks.

> Next, whoever wrote the standard clearly did not understand what
> Ctl.getERROR should do. It should return an OS.SysErr option, not a
> bool!
...
> As one approach to agree with the standard in definition, but
> disagree in spirit, I propose that getERROR raise the appropriate
> OS.SysErr if there is an error, otherwise return false.

Better to make deviant behavior explicit than to hide it.  So we
should either change the type of Socket.Ctl.getERROR or we should add
MLton.Socket.Ctl.getERROR with the correct type.  I think the latter
would be preferable.

> For that matter, the patch I made to MLton has the side benefit of
> already working on Solaris since it will think the getsockopt call
> failed (it did this before too) and raise the exception. All my
> patch does from this point of view, is bring things in line with the
> Solaris approach... ;-) ie: raise also based on the value in &err.

Similarly, I think it would be preferable to fix this bug in MLton on
Solaris, leaving the behavior of the basis library consistent.  It can
easily be fixed with a handler, I think.

So, I propose the following patch.


Index: basis-library/mlton/socket.sml
===================================================================
--- basis-library/mlton/socket.sml	(revision 3991)
+++ basis-library/mlton/socket.sml	(working copy)
@@ -101,5 +101,7 @@
     ; shutdown (TextIO.outFd out, Socket.NO_SENDS))
 
 val fdToSock = Socket.fdToSock
+
+structure Ctl = Socket.CtlExtra
    
 end
Index: basis-library/mlton/socket.sig
===================================================================
--- basis-library/mlton/socket.sig	(revision 3991)
+++ basis-library/mlton/socket.sig	(working copy)
@@ -16,6 +16,11 @@
 	    type t = word
 	 end
 
+      structure Ctl:
+         sig
+            val getERROR: ('af, 'sock_type) Socket.sock -> int
+         end
+
       structure Host:
 	 sig
 	    type t = {name: string}
Index: basis-library/net/socket.sml
===================================================================
--- basis-library/net/socket.sml	(revision 3993)
+++ basis-library/net/socket.sml	(working copy)
@@ -190,12 +190,7 @@
 	 val (getSockOptTimeOpt, _, setSockOptTimeOpt, _) =
 	    make (timeOptLen, marshalTimeOpt, unmarshalTimeOpt)
       end
-   end
 
-structure Ctl =
-   struct
-      open CtlExtra
-
       val getDEBUG = getSockOptBool (Prim.Ctl.SOCKET, Prim.Ctl.DEBUG)
       val setDEBUG = setSockOptBool (Prim.Ctl.SOCKET, Prim.Ctl.DEBUG)
       val getREUSEADDR = getSockOptBool (Prim.Ctl.SOCKET, Prim.Ctl.REUSEADDR)
@@ -216,7 +211,12 @@
       val setRCVBUF = setSockOptInt (Prim.Ctl.SOCKET, Prim.Ctl.RCVBUF)
       fun getTYPE s =
 	 Prim.SOCK.fromInt (getSockOptInt (Prim.Ctl.SOCKET, Prim.Ctl.TYPE) s)
-      val getERROR = getSockOptBool (Prim.Ctl.SOCKET, Prim.Ctl.ERROR)
+      fun getERROR s =
+         getSockOptInt (Prim.Ctl.SOCKET, Prim.Ctl.ERROR) s
+         handle Error.SysErr (_, e) =>
+            case e of
+               NONE => raise Fail "Socket.Ctl.getERROR"
+             | SOME s => s
       local
 	 fun getName (s, f: Prim.sock * pre_sock_addr * int ref -> int) =
 	    let
@@ -233,6 +233,13 @@
       val getATMARK = getIOCtlBool Prim.Ctl.ATMARK
    end
 
+structure Ctl =
+   struct
+      open CtlExtra
+
+      fun getERROR s = 0 < CtlExtra.getERROR s
+   end
+
 fun sameAddr (SA sa1, SA sa2) = sa1 = sa2
 
 fun familyOfAddr (SA sa) = NetHostDB.intToAddrFamily (Prim.familyOfAddr sa)
Index: basis-library/net/socket.sig
===================================================================
--- basis-library/net/socket.sig	(revision 3991)
+++ basis-library/net/socket.sig	(working copy)
@@ -183,6 +183,7 @@
 (* 	  val setSockOptWord:
  * 	     level * optname -> ('af, 'sock_type) sock * word -> unit
  *)
+          val getERROR: ('af, 'sock_type) sock -> int
 	  val getSockOptInt: level * optname -> ('af, 'sock_type) sock -> int
 	  val setSockOptInt:
 	     level * optname -> ('af, 'sock_type) sock * int -> unit