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

fs: minor refactoring #1870

Closed
wants to merge 5 commits into from
Closed
Changes from 1 commit
Commits
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
Prev Previous commit
Next Next commit
fs: minor refactoring
1. Removed a few unnecessary variables to reduce LoC

2. Removed redundant `var` definitions of variables in same function

3. Refactored variables which are defined inside a block and used
outside as well

4. Refactored effect-less code

5. In `rethrow` function, instead of assigning to `err` and throwing
`err`, we can directly throw `backtrace` object itself.

6. Reassigning a defined parameter while also mentioning arguments in
the body is one of the optimization killers. So, changing `callback` to
`callback_` and declaring a new variable called `callback` in the body.
  • Loading branch information
thefourtheye committed Jun 11, 2015
commit 08f2691bcdd6a8b282062cc58b7d91615b6f1bef
57 changes: 29 additions & 28 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,7 @@ function rethrow() {
if (err) {
backtrace.stack = err.name + ': ' + err.message +
backtrace.stack.substr(backtrace.name.length);
err = backtrace;
throw err;
throw backtrace;
}
};
}
Expand Down Expand Up @@ -267,27 +266,25 @@ function ReadFileContext(callback, encoding) {
}

ReadFileContext.prototype.read = function() {
var fd = this.fd;
var size = this.size;
var buffer;
var offset;
var length;

if (size === 0) {
if (this.size === 0) {
buffer = this.buffer = new SlowBuffer(kReadFileBufferLength);
offset = 0;
length = kReadFileBufferLength;
} else {
buffer = this.buffer;
offset = this.pos;
length = size - this.pos;
length = this.size - this.pos;
}

var req = new FSReqWrap();
req.oncomplete = readFileAfterRead;
req.context = this;

binding.read(fd, buffer, offset, length, -1, req);
binding.read(this.fd, buffer, offset, length, -1, req);
};

ReadFileContext.prototype.close = function(err) {
Expand All @@ -302,8 +299,7 @@ function readFileAfterOpen(err, fd) {
var context = this.context;

if (err) {
var callback = context.callback;
callback(err);
context.callback(err);
return;
}

Expand Down Expand Up @@ -417,7 +413,7 @@ fs.readFileSync = function(path, options) {
if (size === 0) {
buffers = [];
} else {
var threw = true;
threw = true;
try {
buffer = new Buffer(size);
threw = false;
Expand All @@ -427,16 +423,18 @@ fs.readFileSync = function(path, options) {
}

var done = false;
var bytesRead;

while (!done) {
var threw = true;
threw = true;
try {
if (size !== 0) {
var bytesRead = fs.readSync(fd, buffer, pos, size - pos);
bytesRead = fs.readSync(fd, buffer, pos, size - pos);
} else {
// the kernel lies about many files.
// Go ahead and try to read some bytes.
buffer = new Buffer(8192);
var bytesRead = fs.readSync(fd, buffer, 0, 8192);
bytesRead = fs.readSync(fd, buffer, 0, 8192);
if (bytesRead) {
buffers.push(buffer.slice(0, bytesRead));
}
Expand Down Expand Up @@ -529,8 +527,8 @@ function modeNum(m, def) {
return undefined;
}

fs.open = function(path, flags, mode, callback) {
callback = makeCallback(arguments[arguments.length - 1]);
fs.open = function(path, flags, mode, callback_) {
var callback = makeCallback(arguments[arguments.length - 1]);
mode = modeNum(mode, 0o666);

if (!nullCheck(path, callback)) return;
Expand Down Expand Up @@ -585,10 +583,12 @@ fs.read = function(fd, buffer, offset, length, position, callback) {

fs.readSync = function(fd, buffer, offset, length, position) {
var legacy = false;
var encoding;

if (!(buffer instanceof Buffer)) {
// legacy string interface (fd, length, position, encoding, callback)
legacy = true;
var encoding = arguments[3];
encoding = arguments[3];

assertEncoding(encoding);

Expand Down Expand Up @@ -623,14 +623,14 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
callback(err, written || 0, buffer);
}

var req = new FSReqWrap();
if (buffer instanceof Buffer) {
// if no position is passed then assume null
if (typeof position === 'function') {
callback = position;
position = null;
}
callback = maybeCallback(callback);
var req = new FSReqWrap();
req.oncomplete = strWrapper;
return binding.writeBuffer(fd, buffer, offset, length, position, req);
}
Expand All @@ -647,7 +647,6 @@ fs.write = function(fd, buffer, offset, length, position, callback) {
length = 'utf8';
}
callback = maybeCallback(position);
var req = new FSReqWrap();
req.oncomplete = bufWrapper;
return binding.writeString(fd, buffer, offset, length, req);
};
Expand Down Expand Up @@ -721,8 +720,10 @@ fs.truncateSync = function(path, len) {
}
// allow error to be thrown, but still close fd.
var fd = fs.openSync(path, 'r+');
var ret;

try {
var ret = fs.ftruncateSync(fd, len);
ret = fs.ftruncateSync(fd, len);
} finally {
fs.closeSync(fd);
}
Expand Down Expand Up @@ -875,7 +876,7 @@ function preprocessSymlinkDestination(path, type, linkPath) {
}
}

fs.symlink = function(destination, path, type_, callback) {
fs.symlink = function(destination, path, type_, callback_) {
var type = (typeof type_ === 'string' ? type_ : null);
var callback = makeCallback(arguments[arguments.length - 1]);

Expand Down Expand Up @@ -968,9 +969,9 @@ if (constants.hasOwnProperty('O_SYMLINK')) {

// prefer to return the chmod error, if one occurs,
// but still try to close, and report closing errors if they occur.
var err, err2;
var err, err2, ret;
try {
var ret = fs.fchmodSync(fd, mode);
ret = fs.fchmodSync(fd, mode);
} catch (er) {
err = er;
}
Expand Down Expand Up @@ -1088,8 +1089,8 @@ fs.futimesSync = function(fd, atime, mtime) {
binding.futimes(fd, atime, mtime);
};

function writeAll(fd, buffer, offset, length, position, callback) {
callback = maybeCallback(arguments[arguments.length - 1]);
function writeAll(fd, buffer, offset, length, position, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);

// write(fd, buffer, offset, length, position, callback)
fs.write(fd, buffer, offset, length, position, function(writeErr, written) {
Expand All @@ -1112,7 +1113,7 @@ function writeAll(fd, buffer, offset, length, position, callback) {
});
}

fs.writeFile = function(path, data, options, callback) {
fs.writeFile = function(path, data, options, callback_) {
var callback = maybeCallback(arguments[arguments.length - 1]);

if (!options || typeof options === 'function') {
Expand Down Expand Up @@ -1627,8 +1628,8 @@ function ReadStream(path, options) {
this.flags = options.flags === undefined ? 'r' : options.flags;
this.mode = options.mode === undefined ? 0o666 : options.mode;

this.start = options.start === undefined ? undefined : options.start;
this.end = options.end === undefined ? undefined : options.end;
this.start = options.start;
this.end = options.end;
this.autoClose = options.autoClose === undefined ? true : options.autoClose;
this.pos = undefined;

Expand Down Expand Up @@ -1790,7 +1791,7 @@ function WriteStream(path, options) {
this.flags = options.flags === undefined ? 'w' : options.flags;
this.mode = options.mode === undefined ? 0o666 : options.mode;

this.start = options.start === undefined ? undefined : options.start;
this.start = options.start;
this.pos = undefined;
this.bytesWritten = 0;

Expand Down