From a33fa923add3c0ed1cd71da08911d82cc80bbfbb Mon Sep 17 00:00:00 2001
From: Albert Slawinski <errendir@gmail.com>
Date: Mon, 8 Sep 2014 15:06:09 -0700
Subject: [PATCH] Apply the code review comments

---
 doc/api/fs.markdown | 20 ++++++++++++++------
 lib/fs.js           | 40 ++++++++++++++--------------------------
 lib/module.js       |  6 +++++-
 3 files changed, 33 insertions(+), 33 deletions(-)

diff --git a/doc/api/fs.markdown b/doc/api/fs.markdown
index 587ce4a06104..2d447eaf4fc6 100644
--- a/doc/api/fs.markdown
+++ b/doc/api/fs.markdown
@@ -84,6 +84,12 @@ site, set the NODE_DEBUG environment variable:
         at Object.<anonymous> (/path/to/script.js:5:1)
         <etc.>
 
+## fs.CreateConfiguredFSObject(options)
+
+Creates and returns a new fs object with the specified options set. The valid options
+are:
+* `throwSafe` to prevent the error creation when the non-existent file is stat-ed
+
 
 ## fs.rename(oldPath, newPath, callback)
 
@@ -168,14 +174,15 @@ Only available on Mac OS X.
 
 Synchronous lchmod(2).
 
-## fs.stat(path, callback, [throwSafe])
+## fs.stat(path, callback)
 
 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.
 
-Setting the throwSafe parameter to true prevents the error creation. If the call
-fails, the first parameter of the callback will be true.
+Setting the throwSafe fs option to true (see [fs.CreateConfiguredFSObject](#fs_class_fs_stats))
+prevents the error creation. If the call fails, the first parameter of the callback will be true.
+This does not prevent the errors when the path contains the null character.
 
 ## fs.lstat(path, callback)
 
@@ -190,12 +197,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, [throwSafe])
+## fs.statSync(path)
 
 Synchronous stat(2). Returns an instance of `fs.Stats`.
 
-Setting the throwSafe parameter to true prevents the error creation. If the call
-fails `fs.statSync` returns `false`.
+Setting the throwSafe fs option to true (see [fs.CreateConfiguredFSObject](#fs_class_fs_stats))
+prevents the error creation. If the call fails `fs.statSync` returns `false`.
+This does not prevent the errors when the path contains the null character.
 
 ## fs.lstatSync(path)
 
diff --git a/lib/fs.js b/lib/fs.js
index 963e87344455..a59798a0de45 100644
--- a/lib/fs.js
+++ b/lib/fs.js
@@ -113,13 +113,15 @@ var CreateConfiguredFSObject = function(options) {
     }
   }
 
-  function nullCheck(path, callback, throwSafe) {
-    if (typeof throwSafe === 'undefined')
-      throwSafe = fs.options.throwSafe;
+  function simpleNullCheck(path) {
+    if (('' + path).indexOf('\u0000') !== -1) {
+      return false;
+    }
+    return true;
+  }
 
+  function nullCheck(path, callback) {
     if (('' + path).indexOf('\u0000') !== -1) {
-      if (throwSafe)
-        return false;
       var er = new Error('Path must be a string without null bytes.');
       if (!callback)
         throw er;
@@ -199,25 +201,20 @@ var CreateConfiguredFSObject = function(options) {
   };
 
   fs.exists = function(path, callback) {
-    if (!nullCheck(path, undefined, true)) {
-      process.nextTick(function() {
+    if (!simpleNullCheck(path)) {
+      process.nextTick(function () {
         callback(false);
       });
       return;
     }
     binding.stat(pathModule._makeLong(path), cb, true);
-    function cb(err, result) {
-      if (callback) {
-        if (err)
-          callback(false);
-        else
-          callback(!!result);
-      }
+    function cb(err, stats) {
+      if (callback) callback(err ? false : !!stats);
     }
   };
 
   fs.existsSync = function(path) {
-    if (!nullCheck(path, undefined, true))
+    if (!simpleNullCheck(path))
       return false;
     if (!binding.stat(pathModule._makeLong(path), undefined, true))
       return false;
@@ -732,14 +729,7 @@ var CreateConfiguredFSObject = function(options) {
     var throwSafe = fs.options.throwSafe;
 
     callback = makeCallback(callback);
-    if (!nullCheck(path, callback)) {
-      if (throwSafe) {
-        process.nextTick(function() {
-          callback(true, false);
-        });
-      }
-      return;
-    }
+    if (!nullCheck(path, callback)) return;
     binding.stat(pathModule._makeLong(path), callback, throwSafe);
   };
 
@@ -755,9 +745,7 @@ var CreateConfiguredFSObject = function(options) {
   fs.statSync = function(path) {
     var throwSafe = fs.options.throwSafe;
 
-    if (!nullCheck(path))
-      return false;
-
+    nullCheck(path);
     return binding.stat(pathModule._makeLong(path), undefined, throwSafe);
   };
 
diff --git a/lib/module.js b/lib/module.js
index 0d3859458eb5..ff07143ac114 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -85,7 +85,11 @@ var _fs = fs.CreateConfiguredFSObject({ throwSafe: true });
 function statPath(path) {
   var fs = NativeModule.require('fs');
 
-  return _fs.statSync(path, true);
+  try {
+    return _fs.statSync(path, true);
+  } catch(err) {
+    return false;
+  }
 }
 
 // check if the directory is a package.json dir