code review 961047: mingw syscall: Sleep functionality added. Resubmitted a...

mingw syscall: Sleep functionality added. Resubmitted as per request from Alex B.

The function ahould be called directly for now rather than from time.Sleep;
syscall.Gettimeofday is called by time.Sleep but it hasn't been implemented on
windows yet.

[Joe Poirier, 2010-04-29 13:45:19]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[brainman, 2010-04-30 01:26:19]

http://codereview.appspot.com/961047/diff/2001/3001
File src/pkg/syscall/syscall_mingw.go (right):

http://codereview.appspot.com/961047/diff/2001/3001#newcode124
src/pkg/syscall/syscall_mingw.go:124: //sys   SleepMilliseconds(msec uint32)
(errno int) = Sleep
Sleep windows API doesn't have return value, this line should be like this:

//sys SleepMilliseconds(msec uint32) = Sleep

http://codereview.appspot.com/961047/diff/2001/3001#newcode124
src/pkg/syscall/syscall_mingw.go:124: //sys   SleepMilliseconds(msec uint32)
(errno int) = Sleep
I do not like name SleepMilliseconds for Sleep win api, maybe call it 'sleep'. I
don't like renaming apis. I'll leave it for Russ to decide.

http://codereview.appspot.com/961047/diff/2001/3001#newcode129
src/pkg/syscall/syscall_mingw.go:129: // TODO(jpoirier): Check on timer
resolution between the different windows
I think this comment is not needed. We won't use "high performance times" now. I
doubt ever. If one day we encounter the problem that we need more precise time,
we know were to look. Less lines for people to look at is better.

[rsc, 2010-04-30 04:24:07]

It looks like for the other architectures I went out of
my way to make syscall.Sleep take nanoseconds.
The same should probably happen here:

//sys   sleep(msec uint32) (errno int)

(add = Sleep if necessary; I don't know whether it is)
and then write a wrapper func Sleep that does nanoseconds.

Russ

[brainman, 2010-04-30 04:32:15]

On 2010/04/30 04:24:07, rsc wrote:

> 
> //sys   sleep(msec uint32) (errno int)
> 
> (add = Sleep if necessary; I don't know whether it is)

Sorry, but no return value.

And do put '= Sleep' at the end, I'm not sure if GetProcAddress is case
sensitive or not, but all win apis are capitalized.

So,

//sys sleep(msec uint32) = Sleep


Alex

[Joe Poirier, 2010-04-30 04:59:17]

On Thu, Apr 29, 2010 at 11:24 PM, Russ Cox <rsc@golang.org> wrote:

> It looks like for the other architectures I went out of
> my way to make syscall.Sleep take nanoseconds.
> The same should probably happen here:
>
> //sys   sleep(msec uint32) (errno int)
>
> (add = Sleep if necessary; I don't know whether it is)
> and then write a wrapper func Sleep that does nanoseconds.
>
> Russ
>

I did include a wrapper function in syscall_mingw.go that takes nanoseconds
as a parameter:

func Sleep(nsec int64) (errno int) {
// TODO(jpoirier): Check on timer resolution between the different windows
// platforms, milliseconds only as far as I know unless the high performance
timers
// are used, which would require much more code and other DLLs.
nsec += 999999 // round up to milliseconds
msec := uint32(nsec / 1e6)
e := SleepMilliseconds(msec)
errno = int(e)
return
}

-joe

[rsc, 2010-04-30 05:28:21]

okay, great, so let's just call the underlying call sleep
instead of SleepMilliseconds and we're done.

[Joe Poirier, 2010-04-30 05:42:54]

Hello rsc, brainman (cc: golang-dev@googlegroups.com),

Please take another look.

[brainman, 2010-04-30 05:51:39]

On 2010/04/30 05:42:54, Joe Poirier wrote:
> Hello rsc, brainman (cc: mailto:golang-dev@googlegroups.com),
> 
> Please take another look.

LGTM.


Alex

[rsc, 2010-04-30 06:08:24]

*** Submitted as http://code.google.com/p/go/source/detail?r=06c46f7e965a ***

syscall: mingw Sleep

R=rsc, brainman
CC=golang-dev
http://codereview.appspot.com/961047

Committer: Russ Cox <rsc@golang.org>