code review 122600043: go.sys/windows: use syscall.Errno for windows errors (issue 122600043 by r@golang.org)

30 views
Skip to first unread message

r...@golang.org

unread,
Aug 14, 2014, 12:50:29 PM8/14/14
to r...@golang.org, golang-co...@googlegroups.com, alex.b...@gmail.com, re...@codereview-hr.appspotmail.com
Reviewers: rsc,

Message:
Hello rsc (cc: golang-co...@googlegroups.com,
alex.b...@gmail.com),

I'd like you to review this change to
https://code.google.com/p/go.sys


Description:
go.sys/windows: use syscall.Errno for windows errors
If we use a local type, it won't compare properly with errors from
the rest of the standard library. Errors are the one type from syscall
that propagates through the system, so it's important to have only
one type for them.

mkerrors_windows.sh is gone, so:
- rename zerrors_windows.go and delete its DO NOT EDIT mark
- delete the contentless zerrors_windows_*.go files

Please review this at https://codereview.appspot.com/122600043/

Affected files (+61, -68 lines):
M windows/errors_windows.go
M windows/mksyscall_windows.go
M windows/syscall.go
R windows/zerrors_windows_386.go
R windows/zerrors_windows_amd64.go
M windows/zsyscall_windows_386.go
M windows/zsyscall_windows_amd64.go
M windows/ztypes_windows.go


Russ Cox

unread,
Aug 14, 2014, 1:16:57 PM8/14/14
to Rob Pike, Russ Cox, golang-co...@googlegroups.com, Alex Brainman, re...@codereview-hr.appspotmail.com
LGTM

r...@golang.org

unread,
Aug 14, 2014, 1:28:23 PM8/14/14
to r...@golang.org, r...@golang.org, alex.b...@gmail.com, golang-co...@googlegroups.com, re...@codereview-hr.appspotmail.com
*** Submitted as
https://code.google.com/p/go/source/detail?r=7e8f794ac00a&repo=sys ***

go.sys/windows: use syscall.Errno for windows errors
If we use a local type, it won't compare properly with errors from
the rest of the standard library. Errors are the one type from syscall
that propagates through the system, so it's important to have only
one type for them.

mkerrors_windows.sh is gone, so:
- rename zerrors_windows.go and delete its DO NOT EDIT mark
- delete the contentless zerrors_windows_*.go files

LGTM=rsc
R=rsc
CC=alex.brainman, golang-codereviews
https://codereview.appspot.com/122600043


https://codereview.appspot.com/122600043/

alex.b...@gmail.com

unread,
Aug 14, 2014, 7:56:57 PM8/14/14
to r...@golang.org, r...@golang.org, golang-co...@googlegroups.com, re...@codereview-hr.appspotmail.com
LGTM

but the build is broken now:

# code.google.com/p/go.sys/windows
./errors_windows.go:11: cannot use ERROR_FILE_NOT_FOUND as type
syscall.Errno in const initializer
./errors_windows.go:12: cannot use ERROR_PATH_NOT_FOUND as type
syscall.Errno in const initializer
./ztypes_windows.go:9: undefined: syscall
./ztypes_windows.go:10: undefined: syscall
./ztypes_windows.go:11: undefined: syscall
./ztypes_windows.go:12: undefined: syscall
./ztypes_windows.go:13: undefined: syscall
./ztypes_windows.go:14: undefined: syscall
./ztypes_windows.go:15: undefined: syscall
./ztypes_windows.go:16: undefined: syscall
./ztypes_windows.go:16: too many errors

You should have gone further and deleted errors_windows.go altogether.
None of consts still remaining in errors_windows.go (with exception of
APPLICATION_ERROR) are actually defined by Windows. We "invented" these
at the start of windows port to have minimal impact on existing Go
packages. I don't think we should keep them around.

Would you like me to try and do that?

Alex

https://codereview.appspot.com/122600043/

Rob Pike

unread,
Aug 14, 2014, 7:57:55 PM8/14/14
to Rob Pike, Alex Brainman, Russ Cox, golang-co...@googlegroups.com, re...@codereview-hr.appspotmail.com
Sure, thanks.

alex.b...@gmail.com

unread,
Aug 14, 2014, 9:08:55 PM8/14/14
to r...@golang.org, r...@golang.org, golang-co...@googlegroups.com, re...@codereview-hr.appspotmail.com
Reply all
Reply to author
Forward
0 new messages