code review 5554057: syscall: add Unix method to TimeSpec, TimeVal

syscall: add Unix method to TimeSpec, TimeVal

Fixes issue 2534

[bradfitz, 2012-01-19 01:56:22]

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

I'd like you to review this change to
https://go.googlecode.com/hg/

[dave_cheney.net, 2012-01-19 02:07:18]

LGTM. Thanks for looking at this. 

Sent from my iPhone

On 19/01/2012, at 12:56, bradfitz@golang.org wrote:

> Reviewers: golang-dev_googlegroups.com,
> 
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
> 
> I'd like you to review this change to
> https://go.googlecode.com/hg/
> 
> 
> Description:
> syscall: add Unix method to TimeSpec, TimeVal
> 
> Fixes issue 2534
> 
> Please review this at http://codereview.appspot.com/5554057/
> 
> Affected files:
>  M src/pkg/syscall/syscall.go
>  M src/pkg/syscall/syscall_windows.go
> 
> 
> Index: src/pkg/syscall/syscall.go
> ===================================================================
> --- a/src/pkg/syscall/syscall.go
> +++ b/src/pkg/syscall/syscall.go
> @@ -29,3 +29,11 @@
> // Single-word zero for use when we need a valid pointer to 0 bytes.
> // See mksyscall.pl.
> var _zero uintptr
> +
> +func (ts *Timespec) Unix() (sec int64, nsec int64) {
> +    return int64(ts.Sec), int64(ts.Nsec)
> +}
> +
> +func (tv *Timeval) Unix() (sec int64, nsec int64) {
> +    return int64(tv.Sec), int64(tv.Usec) * 1000
> +}
> Index: src/pkg/syscall/syscall_windows.go
> ===================================================================
> --- a/src/pkg/syscall/syscall_windows.go
> +++ b/src/pkg/syscall/syscall_windows.go
> @@ -624,6 +624,13 @@
> 
> func (w WaitStatus) TrapCause() int { return -1 }
> 
> +// Timespec is an invented structure on Windows, but here for
> +// consistency with the syscall package for other operating systems.
> +type Timespec struct {
> +    Sec  int64
> +    Nsec int64
> +}
> +
> // TODO(brainman): fix all needed for net
> 
> func Accept(fd Handle) (nfd Handle, sa Sockaddr, err error) { return 0, nil,
EWINDOWS }
> 
> 

[brainman, 2012-01-19 02:40:52]

http://codereview.appspot.com/5554057/diff/3/src/pkg/syscall/syscall_windows.go
File src/pkg/syscall/syscall_windows.go (right):

http://codereview.appspot.com/5554057/diff/3/src/pkg/syscall/syscall_windows....
src/pkg/syscall/syscall_windows.go:629: type Timespec struct {
Please, do not create "invented" structures. I am trying to get rid of them.
syscall package is os specific. If user expects to have syscall.Timespec, I
would like his program to fail to compile.

[bradfitz, 2012-01-19 02:44:56]

On Wed, Jan 18, 2012 at 6:40 PM, <alex.brainman@gmail.com> wrote:

>
> http://codereview.appspot.com/**5554057/diff/3/src/pkg/**
>
syscall/syscall_windows.go<http://codereview.appspot.com/5554057/diff/3/src/pkg/syscall/syscall_windows.go>
> File src/pkg/syscall/syscall_**windows.go (right):
>
> http://codereview.appspot.com/**5554057/diff/3/src/pkg/**
>
syscall/syscall_windows.go#**newcode629<http://codereview.appspot.com/5554057/diff/3/src/pkg/syscall/syscall_windows.go#newcode629>
> src/pkg/syscall/syscall_**windows.go:629: type Timespec struct {
> Please, do not create "invented" structures. I am trying to get rid of
> them. syscall package is os specific. If user expects to have
> syscall.Timespec, I would like his program to fail to compile.


Fair enough.  I was just copying the existing ones.  I didn't know they
were on their way out.

It definitely made the build easier this way, but I can go mess with +build
and Makefiles.

[brainman, 2012-01-19 02:47:10]

I am ok to have it in, if it simplify things somewhere else in our tree. But I
don't see any ...

Alex

[brainman, 2012-01-19 02:48:37]

Sorry, I didn't look at syscall.go. It is LGTM to leave it as is.

Alex

[bradfitz, 2012-01-19 03:05:46]

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

syscall: add Unix method to TimeSpec, TimeVal

Fixes issue 2534

R=golang-dev, dave, alex.brainman
CC=golang-dev
http://codereview.appspot.com/5554057

[bradfitz, 2012-01-19 03:09:14]

Thanks. Once Makefiles go away (soon?) I can come back and do some cleanup
here.

On Wed, Jan 18, 2012 at 6:48 PM, <alex.brainman@gmail.com> wrote:

> Sorry, I didn't look at syscall.go. It is LGTM to leave it as is.
>
> Alex
>
>
http://codereview.appspot.com/**5554057/<http://codereview.appspot.com/5554057/>
>