[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.