Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

io/ioutil: ReadDir: no error if dirname is a file #9789

Closed
larsbutler opened this issue Feb 6, 2015 · 6 comments
Closed

io/ioutil: ReadDir: no error if dirname is a file #9789

larsbutler opened this issue Feb 6, 2015 · 6 comments
Milestone

Comments

@larsbutler
Copy link

Hi, I noticed that if you run ioutil.ReadDir("/some/file.txt"), it succeeds and returns an empty array of os.FileInfo and no error. I would expect an error, since the input is a file, not a dir.

See

func ReadDir(dirname string) ([]os.FileInfo, error) {
.

Equivalent operations in the stdlib of, for example, Python are os.listdir, and this raises an error if you pass in a file instead of a directory.

Is this a bug, or is this the intended behavior? If the latter, why?

If it's a bug I'll be happy to submit a patch.

Thanks!

@ianlancetaylor
Copy link
Contributor

I think it would be reasonable to check that os.File.Readdir and os.File.Readdirnames return an error if used on a non-directory. As far as I can see that should already happens on GNU/Linux. What OS are you using?

@larsbutler
Copy link
Author

@ianlancetaylor
I'm running OSX 10.8.5.

@mikioh mikioh changed the title ioutil.ReadDir: no error if dirname is a file io/ioutil: ReadDir: no error if dirname is a file Feb 6, 2015
@bradfitz
Copy link
Contributor

bradfitz commented Feb 8, 2015

On Linux:

--- PASS: TestReadDirOnFile (0.00s)
        ioutil_test.go:106: Got [], readdirent: not a directory

On OS X:

--- PASS: TestReadDirOnFile (0.00s)
    ioutil_test.go:106: Got [], <nil>

@bradfitz bradfitz self-assigned this Feb 8, 2015
@bradfitz bradfitz added this to the Go1.5 milestone Feb 8, 2015
@bradfitz
Copy link
Contributor

bradfitz commented Feb 8, 2015

syscall.Getdirentries is actually returning EINVAL on OS X, but @rsc added Yosemite work-around code to mask that error away.

With my added logging the syscall func is:

func ReadDirent(fd int, buf []byte) (n int, err error) {
        // Final argument is (basep *uintptr) and the syscall doesn't take nil.                                                                                             
        // 64 bits should be enough. (32 bits isn't even on 386). Since the                                                                                                 
        // actual system call is getdirentries64, 64 is a good guess.                                                                                                       
        // TODO(rsc): Can we use a single global basep for all calls?                                                                                                       
        var base = (*uintptr)(unsafe.Pointer(new(uint64)))
        n, err = Getdirentries(fd, buf, base)

        println("syscall.ReadDirent", n, err)
        if err != nil {
                println("syscall.ReadDirent == " + err.Error())
        }

        // On OS X 10.10 Yosemite, if you have a directory that can be returned                                                                                             
        // in a single getdirentries64 call (for example, a directory with one file),                                                                                       
        // and you read from the directory at EOF twice, you get EOF both times:                                                                                            
        //      fd = open("dir")                                                                                                                                            
        //      getdirentries64(fd) // returns data                                                                                                                         
        //      getdirentries64(fd) // returns 0 (EOF)                                                                                                                      
        //      getdirentries64(fd) // returns 0 (EOF)                                                                                                                      
        //                                                                                                                                                                  
        // But if you remove the file in the middle between the two calls, the                                                                                              
        // second call returns an error instead.                                                                                                                            
        //      fd = open("dir")                                                                                                                                            
        //      getdirentries64(fd) // returns data                                                                                                                         
        //      getdirentries64(fd) // returns 0 (EOF)                                                                                                                      
        //      remove("dir/file")                                                                                                                                          
        //      getdirentries64(fd) // returns ENOENT/EINVAL                                                                                                                
        //                                                                                                                                                                  
        // Whether you get ENOENT or EINVAL depends on exactly what was                                                                                                     
        // in the directory. It is deterministic, just data-dependent.                                                                                                      
        //                                                                                                                                                                  
        // This only happens in small directories. A directory containing more data                                                                                         
        // than fits in a 4k getdirentries64 call will return EOF correctly.                                                                                                
        // (It's not clear if the criteria is that the directory be split across multiple                                                                                   
        // getdirentries64 calls or that it be split across multiple file system blocks.)                                                                                   
        //                                                                                                                                                                  
        // We could change package os to avoid the second read at EOF,                                                                                                      
        // and maybe we should, but that's a bit involved.                                                                                                                  
        // For now, treat the EINVAL/ENOENT as EOF.                                                                                                                         
        if runtime.GOOS == "darwin" && (err == EINVAL || err == ENOENT) {
                err = nil
        }

        return
}

But I thought that the Yosemite issue was fixed?

@bradfitz
Copy link
Contributor

bradfitz commented Feb 8, 2015

The commit that added the Yosemite workaround was a83bbc9

@bradfitz
Copy link
Contributor

bradfitz commented Feb 8, 2015

I've sent https://go-review.googlesource.com/4164

@golang golang locked and limited conversation to collaborators Jun 25, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants