code review 5794073: os: IsExist() and IsNotExist() should also consider ENO...

os: IsNotExist() should also consider ERROR_PATH_NOT_FOUND on Windows
    Also update documentation about IsExist() and IsNotExist(), they are not
    about files only.

[minux1, 2012-03-13 08:53:25]

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

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

[minux1, 2012-03-13 09:00:04]

PTAL, changed the description to:
os: IsNotExist() should also consider ENOTDIR on Windows

IsExist() is not affected by this CL.

Rationale for this CL:
chdir("C:\notexists\notexists2") return the error as ENOTDIR, but the same error
on Unix is ENOENT;
we already include both file and dir errors in IsExist(), so I think we should
also
make IsNotExist() matching behavior in this regard.

This CL also updates the documentation about IsExist() and IsNotExist(), because
they are not only about files.

[rsc, 2012-03-13 13:24:38]

http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go
File src/pkg/os/error_windows.go (right):

http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newc...
src/pkg/os/error_windows.go:13: return err == syscall.EEXIST || err ==
syscall.ERROR_ALREADY_EXISTS ||
Nothing in package syscall for Windows generates EEXIST; please remove.

http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newc...
src/pkg/os/error_windows.go:23: return err == syscall.ENOENT || err ==
syscall.ENOTDIR || err == ErrNotExist
s/ENOENT/ERROR_FILE_NOT_FOUND/
s/ENOTDIR/ERROR_PATH_NOT_FOUND/

Then you don't need the comment.
It's unfortunate that we didn't remove those aliases a while back
but it is too late now.

http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newc...
src/pkg/os/error_windows.go:30: return err == syscall.EACCES || err ==
syscall.EPERM || err == ErrPermission
Nothing on Windows generates EACCES or EPERM; please remove.

[minux1, 2012-03-13 13:42:11]

PTAL.

On Tue, Mar 13, 2012 at 9:24 PM, <rsc@golang.org> wrote:

>
> http://codereview.appspot.com/**5794073/diff/5/src/pkg/os/**
>
error_windows.go<http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go>
> File src/pkg/os/error_windows.go (right):
>
> http://codereview.appspot.com/**5794073/diff/5/src/pkg/os/**
>
error_windows.go#newcode13<http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newcode13>
> src/pkg/os/error_windows.go:**13: return err == syscall.EEXIST || err ==
> syscall.ERROR_ALREADY_EXISTS ||
> Nothing in package syscall for Windows generates EEXIST; please remove.
>
Done.

>
> http://codereview.appspot.com/**5794073/diff/5/src/pkg/os/**
>
error_windows.go#newcode23<http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newcode23>
> src/pkg/os/error_windows.go:**23: return err == syscall.ENOENT || err ==
> syscall.ENOTDIR || err == ErrNotExist
> s/ENOENT/ERROR_FILE_NOT_FOUND/
> s/ENOTDIR/ERROR_PATH_NOT_**FOUND/
>
> Then you don't need the comment.
> It's unfortunate that we didn't remove those aliases a while back
> but it is too late now.
>
Done. Should I add comment about these inaccurate aliases in syscall?

>
> http://codereview.appspot.com/**5794073/diff/5/src/pkg/os/**
>
error_windows.go#newcode30<http://codereview.appspot.com/5794073/diff/5/src/pkg/os/error_windows.go#newcode30>
> src/pkg/os/error_windows.go:**30: return err == syscall.EACCES || err ==
> syscall.EPERM || err == ErrPermission
> Nothing on Windows generates EACCES or EPERM; please remove.
>
Done.

[rsc, 2012-03-14 15:44:46]

LGTM

This is fine for now.  I don't think we need an explicit comment in syscall.

[minux1, 2012-03-14 15:54:49]

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

os: IsNotExist() should also consider ERROR_PATH_NOT_FOUND on Windows
    Also update documentation about IsExist() and IsNotExist(), they are not
    about files only.

R=rsc
CC=golang-dev
http://codereview.appspot.com/5794073