Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Create less IO errors to improve efficiency of require #8189

Closed
wants to merge 1 commit into from
Closed

Create less IO errors to improve efficiency of require #8189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions doc/api/fs.markdown
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,12 +153,15 @@ Only available on Mac OS X.

Synchronous lchmod(2).

## fs.stat(path, callback)
## fs.stat(path, callback, [throwSafe])

Asynchronous stat(2). The callback gets two arguments `(err, stats)` where
`stats` is a [fs.Stats](#fs_class_fs_stats) object. See the [fs.Stats](#fs_class_fs_stats)
section below for more information.

The throwSafe parameter if set to true prevents the error creation. If the call
fails, the first parameter of the callback will be true.

## fs.lstat(path, callback)

Asynchronous lstat(2). The callback gets two arguments `(err, stats)` where
Expand All @@ -172,10 +175,13 @@ Asynchronous fstat(2). The callback gets two arguments `(err, stats)` where
`stats` is a `fs.Stats` object. `fstat()` is identical to `stat()`, except that
the file to be stat-ed is specified by the file descriptor `fd`.

## fs.statSync(path)
## fs.statSync(path, [throwSafe])

Synchronous stat(2). Returns an instance of `fs.Stats`.

The throwSafe parameter if set to true prevents the error creation. If the call
fails, the first parameter of the callback will be true.

## fs.lstatSync(path)

Synchronous lstat(2). Returns an instance of `fs.Stats`.
Expand Down
66 changes: 43 additions & 23 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,14 +114,16 @@ function assertEncoding(encoding) {
}
}

function nullCheck(path, callback) {
function nullCheck(path, callback, throwSafe) {
if (('' + path).indexOf('\u0000') !== -1) {
var er = new Error('Path must be a string without null bytes.');
if (!callback)
throw er;
process.nextTick(function() {
callback(er);
});
if (!throwSafe) {
var er = new Error('Path must be a string without null bytes.');
if (!callback)
throw er;
process.nextTick(function() {
callback(er);
});
}
return false;
}
return true;
Expand Down Expand Up @@ -162,21 +164,30 @@ fs.Stats.prototype.isSocket = function() {
};

fs.exists = function(path, callback) {
if (!nullCheck(path, cb)) return;
binding.stat(pathModule._makeLong(path), cb);
function cb(err, stats) {
if (callback) callback(err ? false : true);
if (!nullCheck(path, undefined, true)) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are you setting cb as undefined? fs.exists() is async, and if the callback isn't passed then it should throw that the callback wasn't passed.

process.nextTick(function() {
cb(true, false);
});
return;
}
binding.stat(pathModule._makeLong(path), cb, true);
function cb(err, result) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like that we are creating a closure every time we enter this function. Could we just move the check above and use it right before cb?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm afraid I don't see how to create less closure without copying the body of the cb into the body of the first if statement. We can make the cb function a variable, but it will have to be defined at the beginning.

if (callback) {
if (err) {
callback(false);
} else {
callback(!!result);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove extraneous curly brackets in the inner if/else.

}
}
};

fs.existsSync = function(path) {
try {
nullCheck(path);
binding.stat(pathModule._makeLong(path));
return true;
} catch (e) {
if (!nullCheck(path, undefined, true))
return false;
}
if (!binding.stat(pathModule._makeLong(path), undefined, true))
return false;
return true;
};

fs.readFile = function(path, options, callback_) {
Expand Down Expand Up @@ -675,10 +686,17 @@ fs.lstat = function(path, callback) {
binding.lstat(pathModule._makeLong(path), callback);
};

fs.stat = function(path, callback) {
fs.stat = function(path, callback, throwSafe) {
callback = makeCallback(callback);
if (!nullCheck(path, callback)) return;
binding.stat(pathModule._makeLong(path), callback);
if (!nullCheck(path, callback, throwSafe)) {
if (throwSafe) {
process.nextTick(function() {
callback(true, false);
});
}
return;
}
binding.stat(pathModule._makeLong(path), callback, throwSafe);
};

fs.fstatSync = function(fd) {
Expand All @@ -690,9 +708,11 @@ fs.lstatSync = function(path) {
return binding.lstat(pathModule._makeLong(path));
};

fs.statSync = function(path) {
nullCheck(path);
return binding.stat(pathModule._makeLong(path));
fs.statSync = function(path, throwSafe) {
if (!nullCheck(path, undefined, throwSafe))
return false;

return binding.stat(pathModule._makeLong(path), undefined, throwSafe);
};

fs.readlink = function(path, callback) {
Expand Down
6 changes: 2 additions & 4 deletions lib/module.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,8 @@ var debug = Module._debug;

function statPath(path) {
var fs = NativeModule.require('fs');
try {
return fs.statSync(path);
} catch (ex) {}
return false;

return fs.statSync(path, true);
}

// check if the directory is a package.json dir
Expand Down
Loading