[MLton] cvs commit: MAIL: PosixError.SysCall changes

Stephen Weeks MLton@mlton.org
Sat, 1 May 2004 18:03:20 -0700


>   Replaced most PosixError.* uses with calls to PosixError.SysCall.*.

Looks great!  Here are few functions that looked odd to me while
reading the new code.

Posix.Process.pause
	Why is there a FIXME comment?

Posix.FileSys.{fstat,lstat,stat}
	Shouldn't these include the construction of the ST.stat 
	within the critical section (i.e. use syscall instead of
	simple)?

OS.IO.poll
	It seems like the array read after the call should be in the
	critical section. 
	Also, I hate to throw another wrinkle in things, but it looks
	to me like the restarted system call in this case should be
	slightly different.  The timeOut should go down by the amount
	of time already waited until the interrupt arrived.  I'm not
	sure if this is practical, though.

NetHostDB.getHostName.
	It seems like the array read after the call should be in the
	critical section.

One general worry I have is the use of {restart = false} to optimize
system calls which are believed to never return EINTR.  I'm not
convinced that the speed/code-size gain is worth the potential
problems caused by missing a case due to incorrect man pages or
different behavior on another platform.  It seems more stable to err
conservatively and use the restarting version everywhere.  It would
also cut the number of special case functions in half, but that's
minor compared to the correctness argument.