os: remove special casing of NUL in Windows file operations

Some file operations, notably Stat and Mkdir, special cased their
behavior when operating on a file named "NUL" (case-insensitive).
This check failed to account for the many other names of the NUL
device, as well as other non-NUL device files: "./nul", "//./nul",
"nul.txt" (on some Windows versions), "con", etc.

Remove the special case.

os.Mkdir("NUL") now returns no error. This is consonant with the
operating system's behavior: CreateDirectory("NUL") succeeds, as
does "MKDIR NUL" on the command line.

os.Stat("NUL") now follows the existing path for FILE_TYPE_CHAR devices,
returning a FileInfo which correctly reports the file as being a
character device.

os.Stat and os.File.Stat have common elements of their logic unified.

For #24482.
For #24556.
For #56217.

Change-Id: I7e70f45901127c9961166dd6dbfe0c4a10b4ab64
Reviewed-on: https://go-review.googlesource.com/c/go/+/448897
Run-TryBot: Damien Neil <[email protected]>
Reviewed-by: Bryan Mills <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Quim Muntal <[email protected]>

Author: neild

src/os/file.go

@@ -254,9 +254,6 @@ func (f *File) WriteString(s string) (n int, err error) {
 // bits (before umask).
 // If there is an error, it will be of type *PathError.
 func Mkdir(name string, perm FileMode) error {
-	if runtime.GOOS == "windows" && isWindowsNulName(name) {
-		return &PathError{Op: "mkdir", Path: name, Err: syscall.ENOTDIR}
-	}
 	longName := fixLongPath(name)
 	e := ignoringEINTR(func() error {
 		return syscall.Mkdir(longName, syscallMode(perm))
@@ -591,24 +588,6 @@ func (f *File) SyscallConn() (syscall.RawConn, error) {
 	return newRawConn(f)
 }
 
-// isWindowsNulName reports whether name is os.DevNull ('NUL') on Windows.
-// True is returned if name is 'NUL' whatever the case.
-func isWindowsNulName(name string) bool {
-	if len(name) != 3 {
-		return false
-	}
-	if name[0] != 'n' && name[0] != 'N' {
-		return false
-	}
-	if name[1] != 'u' && name[1] != 'U' {
-		return false
-	}
-	if name[2] != 'l' && name[2] != 'L' {
-		return false
-	}
-	return true
-}
-
 // DirFS returns a file system (an fs.FS) for the tree of files rooted at the directory dir.
 //
 // Note that DirFS("/prefix") only guarantees that the Open calls it makes to the

src/os/os_test.go

@@ -2012,18 +2012,8 @@ func TestSameFile(t *testing.T) {
 	}
 }
 
-func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo, ignoreCase bool) {
+func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo) {
 	pre := fmt.Sprintf("%s(%q): ", statname, devNullName)
-	name := filepath.Base(devNullName)
-	if ignoreCase {
-		if strings.ToUpper(fi.Name()) != strings.ToUpper(name) {
-			t.Errorf(pre+"wrong file name have %v want %v", fi.Name(), name)
-		}
-	} else {
-		if fi.Name() != name {
-			t.Errorf(pre+"wrong file name have %v want %v", fi.Name(), name)
-		}
-	}
 	if fi.Size() != 0 {
 		t.Errorf(pre+"wrong file size have %d want 0", fi.Size())
 	}
@@ -2038,7 +2028,7 @@ func testDevNullFileInfo(t *testing.T, statname, devNullName string, fi FileInfo
 	}
 }
 
-func testDevNullFile(t *testing.T, devNullName string, ignoreCase bool) {
+func testDevNullFile(t *testing.T, devNullName string) {
 	f, err := Open(devNullName)
 	if err != nil {
 		t.Fatalf("Open(%s): %v", devNullName, err)
@@ -2049,17 +2039,21 @@ func testDevNullFile(t *testing.T, devNullName string, ignoreCase bool) {
 	if err != nil {
 		t.Fatalf("Stat(%s): %v", devNullName, err)
 	}
-	testDevNullFileInfo(t, "f.Stat", devNullName, fi, ignoreCase)
+	testDevNullFileInfo(t, "f.Stat", devNullName, fi)
 
 	fi, err = Stat(devNullName)
 	if err != nil {
 		t.Fatalf("Stat(%s): %v", devNullName, err)
 	}
-	testDevNullFileInfo(t, "Stat", devNullName, fi, ignoreCase)
+	testDevNullFileInfo(t, "Stat", devNullName, fi)
 }
 
 func TestDevNullFile(t *testing.T) {
-	testDevNullFile(t, DevNull, false)
+	testDevNullFile(t, DevNull)
+	if runtime.GOOS == "windows" {
+		testDevNullFile(t, "./nul")
+		testDevNullFile(t, "//./nul")
+	}
 }
 
 var testLargeWrite = flag.Bool("large_write", false, "run TestLargeWriteToConsole test that floods console with output")

src/os/os_windows_test.go

@@ -891,10 +891,6 @@ func TestOneDrive(t *testing.T) {
 }
 
 func TestWindowsDevNullFile(t *testing.T) {
-	testDevNullFile(t, "NUL", true)
-	testDevNullFile(t, "nul", true)
-	testDevNullFile(t, "Nul", true)
-
 	f1, err := os.Open("NUL")
 	if err != nil {
 		t.Fatal(err)
@@ -922,6 +918,30 @@ func TestWindowsDevNullFile(t *testing.T) {
 	}
 }
 
+func TestFileStatNUL(t *testing.T) {
+	f, err := os.Open("NUL")
+	if err != nil {
+		t.Fatal(err)
+	}
+	fi, err := f.Stat()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := fi.Mode(), os.ModeDevice|os.ModeCharDevice|0666; got != want {
+		t.Errorf("Open(%q).Stat().Mode() = %v, want %v", "NUL", got, want)
+	}
+}
+
+func TestStatNUL(t *testing.T) {
+	fi, err := os.Stat("NUL")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if got, want := fi.Mode(), os.ModeDevice|os.ModeCharDevice|0666; got != want {
+		t.Errorf("Stat(%q).Mode() = %v, want %v", "NUL", got, want)
+	}
+}
+
 // TestSymlinkCreation verifies that creating a symbolic link
 // works on Windows when developer mode is active.
 // This is supported starting Windows 10 (1703, v10.0.14972).
@@ -1232,19 +1252,3 @@ func TestWindowsReadlink(t *testing.T) {
 	mklink(t, "relfilelink", "file")
 	testReadlink(t, "relfilelink", "file")
 }
-
-// os.Mkdir(os.DevNull) fails.
-func TestMkdirDevNull(t *testing.T) {
-	err := os.Mkdir(os.DevNull, 777)
-	oserr, ok := err.(*fs.PathError)
-	if !ok {
-		t.Fatalf("error (%T) is not *fs.PathError", err)
-	}
-	errno, ok := oserr.Err.(syscall.Errno)
-	if !ok {
-		t.Fatalf("error (%T) is not syscall.Errno", oserr)
-	}
-	if errno != syscall.ENOTDIR {
-		t.Fatalf("error %d is not syscall.ENOTDIR", errno)
-	}
-}

src/os/stat_windows.go

@@ -16,40 +16,18 @@ func (file *File) Stat() (FileInfo, error) {
 	if file == nil {
 		return nil, ErrInvalid
 	}
-
 	if file.isdir() {
 		// I don't know any better way to do that for directory
 		return Stat(file.dirinfo.path)
 	}
-	if isWindowsNulName(file.name) {
-		return &devNullStat, nil
-	}
-
-	ft, err := file.pfd.GetFileType()
-	if err != nil {
-		return nil, &PathError{Op: "GetFileType", Path: file.name, Err: err}
-	}
-	switch ft {
-	case syscall.FILE_TYPE_PIPE, syscall.FILE_TYPE_CHAR:
-		return &fileStat{name: basename(file.name), filetype: ft}, nil
-	}
-
-	fs, err := newFileStatFromGetFileInformationByHandle(file.name, file.pfd.Sysfd)
-	if err != nil {
-		return nil, err
-	}
-	fs.filetype = ft
-	return fs, err
+	return statHandle(file.name, file.pfd.Sysfd)
 }
 
 // stat implements both Stat and Lstat of a file.
 func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) {
 	if len(name) == 0 {
 		return nil, &PathError{Op: funcname, Path: name, Err: syscall.Errno(syscall.ERROR_PATH_NOT_FOUND)}
 	}
-	if isWindowsNulName(name) {
-		return &devNullStat, nil
-	}
 	namep, err := syscall.UTF16PtrFromString(fixLongPath(name))
 	if err != nil {
 		return nil, &PathError{Op: funcname, Path: name, Err: err}
@@ -91,14 +69,34 @@ func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) {
 	}
 
 	// Finally use CreateFile.
-	h, err := syscall.CreateFile(namep, 0, 0, nil,
-		syscall.OPEN_EXISTING, createFileAttrs, 0)
+	h, err := syscall.CreateFile(namep,
+		syscall.GENERIC_READ,
+		syscall.FILE_SHARE_READ|syscall.FILE_SHARE_WRITE,
+		nil,
+		syscall.OPEN_EXISTING,
+		createFileAttrs, 0)
 	if err != nil {
 		return nil, &PathError{Op: "CreateFile", Path: name, Err: err}
 	}
 	defer syscall.CloseHandle(h)
+	return statHandle(name, h)
+}
 
-	return newFileStatFromGetFileInformationByHandle(name, h)
+func statHandle(name string, h syscall.Handle) (FileInfo, error) {
+	ft, err := syscall.GetFileType(h)
+	if err != nil {
+		return nil, &PathError{Op: "GetFileType", Path: name, Err: err}
+	}
+	switch ft {
+	case syscall.FILE_TYPE_PIPE, syscall.FILE_TYPE_CHAR:
+		return &fileStat{name: basename(name), filetype: ft}, nil
+	}
+	fs, err := newFileStatFromGetFileInformationByHandle(name, h)
+	if err != nil {
+		return nil, err
+	}
+	fs.filetype = ft
+	return fs, err
 }
 
 // statNolog implements Stat for Windows.

src/os/types_windows.go

@@ -110,9 +110,6 @@ func (fs *fileStat) Size() int64 {
 }
 
 func (fs *fileStat) Mode() (m FileMode) {
-	if fs == &devNullStat {
-		return ModeDevice | ModeCharDevice | 0666
-	}
 	if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 {
 		m |= 0444
 	} else {
@@ -204,15 +201,6 @@ func (fs *fileStat) saveInfoFromPath(path string) error {
 	return nil
 }
 
-// devNullStat is fileStat structure describing DevNull file ("NUL").
-var devNullStat = fileStat{
-	name: DevNull,
-	// hopefully this will work for SameFile
-	vol:   0,
-	idxhi: 0,
-	idxlo: 0,
-}
-
 func sameFile(fs1, fs2 *fileStat) bool {
 	e := fs1.loadFileId()
 	if e != nil {