From dd15813bf6c192bff322c93191ac34e28abddc99 Mon Sep 17 00:00:00 2001 From: Albert Siddhartha Slawinski Date: Tue, 2 Sep 2014 11:15:36 -0700 Subject: [PATCH] Apply the code review comments --- doc/api/fs.markdown | 8 ++++---- lib/fs.js | 31 ++++++++++++++++++------------- src/node_file.cc | 30 ++++++++++++++++-------------- src/node_file.h | 6 +++--- 4 files changed, 41 insertions(+), 34 deletions(-) diff --git a/doc/api/fs.markdown b/doc/api/fs.markdown index 36e0a32ed5bf..380820cb39e0 100644 --- a/doc/api/fs.markdown +++ b/doc/api/fs.markdown @@ -153,13 +153,13 @@ Only available on Mac OS X. Synchronous lchmod(2). -## fs.stat(path, callback, [throw_safe]) +## 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 throw_safe parameter if set to true prevents the error creation. If the call +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) @@ -175,11 +175,11 @@ 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, [throw_safe]) +## fs.statSync(path, [throwSafe]) Synchronous stat(2). Returns an instance of `fs.Stats`. -The throw_safe parameter if set to true prevents the error creation. If the call +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) diff --git a/lib/fs.js b/lib/fs.js index acbd877a5653..381973846fa3 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -114,9 +114,9 @@ function assertEncoding(encoding) { } } -function nullCheck(path, callback, throw_safe) { +function nullCheck(path, callback, throwSafe) { if (('' + path).indexOf('\u0000') !== -1) { - if (!throw_safe) { + if (!throwSafe) { var er = new Error('Path must be a string without null bytes.'); if (!callback) throw er; @@ -165,7 +165,9 @@ fs.Stats.prototype.isSocket = function() { fs.exists = function(path, callback) { if (!nullCheck(path, undefined, true)) { - cb(true, false); + process.nextTick(function() { + cb(true, false); + }) return; } binding.stat(pathModule._makeLong(path), cb, true); @@ -181,8 +183,11 @@ fs.exists = function(path, callback) { }; fs.existsSync = function(path) { - return (nullCheck(path, undefined, true)) && - (!!binding.stat(pathModule._makeLong(path), undefined, true)); + 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_) { @@ -681,17 +686,17 @@ fs.lstat = function(path, callback) { binding.lstat(pathModule._makeLong(path), callback); }; -fs.stat = function(path, callback, throw_safe) { +fs.stat = function(path, callback, throwSafe) { callback = makeCallback(callback); - if (!nullCheck(path, callback, throw_safe)) { - if (throw_safe) { + if (!nullCheck(path, callback, throwSafe)) { + if (throwSafe) { process.nextTick(function() { callback(true, false); }); } return; } - binding.stat(pathModule._makeLong(path), callback, throw_safe); + binding.stat(pathModule._makeLong(path), callback, throwSafe); }; fs.fstatSync = function(fd) { @@ -703,11 +708,11 @@ fs.lstatSync = function(path) { return binding.lstat(pathModule._makeLong(path)); }; -fs.statSync = function(path, throw_safe) { - if (!nullCheck(path, undefined, throw_safe)) { +fs.statSync = function(path, throwSafe) { + if (!nullCheck(path, undefined, throwSafe)) return false; - } - return binding.stat(pathModule._makeLong(path), undefined, throw_safe); + + return binding.stat(pathModule._makeLong(path), undefined, throwSafe); }; fs.readlink = function(path, callback) { diff --git a/src/node_file.cc b/src/node_file.cc index e64e98ee0387..d6589ab03225 100644 --- a/src/node_file.cc +++ b/src/node_file.cc @@ -109,7 +109,7 @@ static void After(uv_fs_t *req) { if (req->result == -1) { // If the request doesn't have a path parameter set. - if (req_node->throw_safe == 1) { + if (req_node->throwSafe == 1) { argc = 2; argv[0] = Local::New(True()); argv[1] = Local::New(False()); @@ -236,7 +236,8 @@ struct fs_req_wrap { }; -#define ASYNC_DEST_CALL(func, callback, dest_path, throw_safe_val, ...) \ +#define ASYNC_DEST_CALL(func, callback, dest_path, \ + throwSafeValue, ...) \ FSReqWrap* req_wrap; \ char* dest_str = (dest_path); \ int dest_len = dest_str == NULL ? 0 : strlen(dest_str); \ @@ -248,7 +249,7 @@ struct fs_req_wrap { dest_str, \ dest_len + 1); \ } \ - req_wrap->req_.throw_safe = throw_safe_val; \ + req_wrap->req_.throwSafe = throwSafeValue; \ int r = uv_fs_##func(uv_default_loop(), \ &req_wrap->req_.req, \ __VA_ARGS__, \ @@ -264,18 +265,19 @@ struct fs_req_wrap { } \ return scope.Close(req_wrap->object_); -#define ASYNC_CALL(func, callback, throw_safe_val, ...) \ - ASYNC_DEST_CALL(func, callback, NULL, throw_safe_val, __VA_ARGS__) \ +#define ASYNC_CALL(func, callback, throwSafeValue, ...) \ + ASYNC_DEST_CALL(func, callback, NULL, throwSafeValue, \ + __VA_ARGS__) \ -#define SYNC_DEST_CALL(func, path, dest, throw_safe_val, ...) \ +#define SYNC_DEST_CALL(func, path, dest, throwSafeValue, ...) \ fs_req_wrap req_wrap; \ - req_wrap.req.throw_safe = throw_safe_val; \ + req_wrap.req.throwSafe = throwSafeValue; \ int result = uv_fs_##func(uv_default_loop(), \ &req_wrap.req.req, \ __VA_ARGS__, \ NULL); \ if (result < 0) { \ - if (req_wrap.req.throw_safe == 1) { \ + if (req_wrap.req.throwSafe == 1) { \ return False(); \ } \ int code = uv_last_error(uv_default_loop()).code; \ @@ -289,8 +291,8 @@ struct fs_req_wrap { } \ } \ -#define SYNC_CALL(func, path, throw_safe_val, ...) \ - SYNC_DEST_CALL(func, path, NULL, throw_safe_val, __VA_ARGS__) \ +#define SYNC_CALL(func, path, throwSafeValue, ...) \ + SYNC_DEST_CALL(func, path, NULL, throwSafeValue, __VA_ARGS__) \ #define SYNC_REQ req_wrap.req.req @@ -418,15 +420,15 @@ static Handle Stat(const Arguments& args) { node::Utf8Value path(args[0]); - int throw_safe = 0; + int throwSafe = 0; if (args[2]->IsTrue()) { - throw_safe = 1; + throwSafe = 1; } if (args[1]->IsFunction()) { - ASYNC_CALL(stat, args[1], throw_safe, *path) + ASYNC_CALL(stat, args[1], throwSafe, *path) } else { - SYNC_CALL(stat, *path, throw_safe, *path) + SYNC_CALL(stat, *path, throwSafe, *path) return scope.Close( BuildStatsObject(static_cast(SYNC_REQ.ptr))); } diff --git a/src/node_file.h b/src/node_file.h index eaa6d8741fa5..a57a1238ee6a 100644 --- a/src/node_file.h +++ b/src/node_file.h @@ -27,11 +27,11 @@ namespace node { -typedef struct node_fs_s { +struct node_fs_t { uv_fs_t req; - int throw_safe; + int throwSafe; void* data; -} node_fs_t; +}; class File { public: