fs: read whole file only in regular files

[santigimeno]

• Trying to fix bug fs.readFile[Sync]("/dev/stdin") does not always read the entire file #1070. Tests still pass in my machine but I haven't been able to reproduce the original bug so I don't know it this fixes anything at all.

[santigimeno]

Finally I could reproduce it in a FreeBSD machine.
Force pushed with fixes for readFile and readFileSync and a couple of tests

[Fishrock123]

r=@piscisaureus?

[piscisaureus]

I don't like landing a 100.000 line bogus text file.
Other than that, lgtm, although I'd prefer another one @iojs/collaborators to review this patch too.

[Fishrock123]

I don't like landing a 100.000 line bogus text file.

Yeah, I didn't really like that either. Maybe we can just generate one in the tests?

[indutny]

I think this is what we usually do in such cases.

[santigimeno]

Updated. Now the file is created inside the tests

[santigimeno]

rebased to latest v1.x and reworded the commit log

[piscisaureus]

https://jenkins-iojs.nodesource.com/job/iojs+any-pr+multi/278/

[piscisaureus]

@santigimeno
The PR looks good to me.
I have one more question though: why is that change to test-fs-sync-fd-leak necessary?

[santigimeno]

@piscisaureus That test was relying on readFileSync to call fstatSync but after the changes it calls statSync

[piscisaureus]

@santigimeno

That test was relying on readFileSync to call fstatSync but after the changes it calls statSync

I missed that, but I also do not agree. By changing to statSync() the operation is now racy, e.g. the file that gets stat()ed might be another file than the one that is opened. I don't think there is a good reason to switch to statSync, so preferrably just use fstatSync.

[piscisaureus]

this should be st = fs.fstatSync(fd);

[santigimeno]

@piscisaureus You're right. Originally I was using fstatSync but I changed to statSync because readfile-zero-byte-liar test was failing and thought there was a problem with fstatSync.

PR updated to use fstatSync and a fix to readfile-zero-byte-liar test

[piscisaureus]

Thanks @santigimeno. Landed in iojs:e2c9040...iojs:5ecdc03