Initial set of fs.promises APIs with documentation and one
benchmark.
PR-URL: https://github.com/nodejs/node/pull/18297
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Problem:
Node implements fs.readFile as:
- a call to stat, then
- a C++ -> libuv request to read the entire file using the stat size
Why is this bad?
The effect is to place on the libuv threadpool a potentially-large
read request, occupying the libuv thread until it completes.
While readFile certainly requires buffering the entire file contents,
it can partition the read into smaller buffers
(as is done on other read paths)
along the way to avoid threadpool exhaustion.
If the file is relatively large or stored on a slow medium, reading
the entire file in one shot seems particularly harmful,
and presents a possible DoS vector.
Solution:
Partition the read into multiple smaller requests.
Considerations:
1. Correctness
I don't think partitioning the read like this raises
any additional risk of read-write races on the FS.
If the application is concurrently readFile'ing and modifying the file,
it will already see funny behavior. Though libuv uses preadv where
available, this doesn't guarantee read atomicity in the presence of
concurrent writes.
2. Performance
Downside: Partitioning means that a single large readFile will
require into many "out and back" requests to libuv,
introducing overhead.
Upside: In between each "out and back", other work pending on the
threadpool can take a turn.
In short, although partitioning will slow down a large request,
it will lead to better throughput if the threadpool is handling
more than one type of request.
Fixes: https://github.com/nodejs/node/issues/17047
PR-URL: https://github.com/nodejs/node/pull/17054
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
lchown and lchownSync were opening file descriptors without
closing them. Looks like it has been that way for 7 years.
Does anyone actually use these functions?
PR-URL: https://github.com/nodejs/node/pull/18329
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
* Collect the error context in both JS and C++, then throw
the error in JS
* Test that the errors thrown from fs.close and fs.closeSync
includes the correct error code, error number and syscall
properties
PR-URL: https://github.com/nodejs/node/pull/17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
- Simplify the SyncCall template function, only collect error
number and syscall in the C++ layer and collect the rest of context
in JS for flexibility.
- Remove the stringFromPath JS helper now that the unprefixed path is
directly put into the context before the binding is invoked with the
prefixed path.
- Validate more properties in fs.access tests.
PR-URL: https://github.com/nodejs/node/pull/17338
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
There two similar error codes in lib: "ERR_VALUE_OUT_OF_RANGE"
and "ERR_OUT_OF_RANGE". This change is to reduce them into
"ERR_VALUE_OUT_OF_RANGE"
Fixes: https://github.com/nodejs/node/issues/17603
PR-URL: https://github.com/nodejs/node/pull/17648
Fixes: https://github.com/nodejs/node/issues/17603
Reviewed-By: Timothy Gu <timothygu99@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
- Migrate the type check of path to ERR_INVALID_ARG_TYPE
- Add template counterparts of ASYNC_CALL, ASYNC_DEST_CALL,
SYNC_CALL, SYNC_DEST_CALL
- Port StringFromPath and UVException to JavaScript
- Migrate the access binding to collect the error context in C++,
then throw the error in JS
PR-URL: https://github.com/nodejs/node/pull/17160
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Replaced _readableState.highWaterMark with a .readableHighWaterMark
getter and _writableState.highWaterMark with a .writableHighWaterMark
getter.
The getters are non-enumerable because they break some prototype
manipulation that happen in the ecosystem.
Ref: https://github.com/nodejs/node/issues/445.
PR-URL: https://github.com/nodejs/node/pull/12860
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The initials of expected in TypeError[ERR_INVALID_ARG_TYPE]
are inconsistent. This change is to unify them.
PR-URL: https://github.com/nodejs/node/pull/16401
Fixes: https://github.com/nodejs/node/issues/16383
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Make the `uv_fs_realpath()` binding (which calls the libc `realpath()`
on UNIX and `GetFinalPathNameByHandle()` on Windows) available as the
`fs.realpath.native()` and `fs.realpathSync.native()` functions.
The binding was already available as `process.binding('fs').realpath`
but was not exposed or tested - and partly broken as a result.
Fixes: https://github.com/nodejs/node/issues/8715
PR-URL: https://github.com/nodejs/node/pull/15776
Refs: https://github.com/nodejs/node/pull/7899
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The comment suggests that the subsequent code could by DRYed up, due to
simply passing arguments along. However, in the commits since then, this
no longer appears to apply, and so the comment is now confusing with
respect to the current code.
PR-URL: https://github.com/nodejs/node/pull/16285
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This change is to unify the declaration for constants into using
destructuring on the top-level-module scope, reducing some redundant
code.
PR-URL: https://github.com/nodejs/node/pull/16063
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/15990
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Refactor close() to use destroy() and not vice versa in ReadStream.
Avoid races between WriteStream.close and WriteStream.write, by aliasing
close to end().
Fixes: https://github.com/nodejs/node/issues/2006
PR-URL: https://github.com/nodejs/node/pull/15407
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
covert lib/fs.js over to using lib/internal/errors.js
i have not addressed the cases that use errnoException(),
for reasons described in GH-12926
- throw the ERR_INVALID_CALLBACK error
when the the callback is invalid
- replace the ['object', 'string'] with
['string', 'object'] in the error constructor call,
to better match the previous err msg
in the getOptions() function
- add error ERR_VALUE_OUT_OF_RANGE in lib/internal/errors.js,
this error is thrown when a numeric value is out of range
- document the ERR_VALUE_OUT_OF_RANGE err in errors.md
- correct the expected args, in the error thrown in the function
fs._toUnixTimestamp() to ['Date', 'time in seconds'] (lib/fs.js)
- update the listener error type in the fs.watchFile() function,
from Error to TypeError (lib/fs.js)
- update errors from ERR_INVALID_OPT_VALUE to ERR_INVALID_ARG_TYPE
in the functions fs.ReadStream() and fs.WriteStream(),
for the cases of range errors use the new error:
ERR_VALUE_OUT_OF_RANGE (lib/fs.js)
PR-URL: https://github.com/nodejs/node/pull/15043
Refs: https://github.com/nodejs/node/issues/11273
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Many callbacks appear to be invoked with `this` set to `undefined`
including `fs.stat()`, `fs.lstat()`, and `fs.fstat()`.
However, some such as `fs.open()` and `fs.mkdtemp()` invoke their
callbacks with `this` set to `null`. Change to `undefined`.
PR-URL: https://github.com/nodejs/node/pull/14645
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
* tryStatSync should not return any value
If the function threw, it would never reach that code path.
* only use try catch if necessary
lchmodSync does not need the second try catch in most cases.
PR-URL: https://github.com/nodejs/node/pull/14055
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
In anticipation of stricter linting for indentation, remove instances of
extra indentation that will be flagged by the new rules.
PR-URL: https://github.com/nodejs/node/pull/14090
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
In anticipation of stricter linting for indentation issues, modify
ternary operators in lib that do not conform with the expected ESLint
settings.
PR-URL: https://github.com/nodejs/node/pull/14078
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/13741
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
https://github.com/nodejs/node/pull/11225 introduce an unnecessary
bind() when closing a stream. This PR replaces that bind() with a
top-level function.
PR-URL: https://github.com/nodejs/node/pull/13474
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Evan Lucas <evanlucas@me.com>
The method used has code duplication but is the most performant
PR-URL: https://github.com/nodejs/node/pull/12818
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Adds destroy() and _destroy() methods to Readable, Writable, Duplex
and Transform. It also standardizes the behavior and the implementation
of destroy(), which has been inconsistent in userland and core.
This PR also updates all the subsystems of core to use the new
destroy().
PR-URL: https://github.com/nodejs/node/pull/12925
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Calvin Metcalf <calvin.metcalf@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This reverts 4cb5f3daa3
Based on community feedback I think we should consider reverting this
change. We should explore how this could be solved via linting rules.
Refs: https://github.com/nodejs/node/pull/12562
PR-URL: https://github.com/nodejs/node/pull/12976
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
This is a small refactor to make an object more readable (IMO).
Yeah, I spent a bit longer looking at the code and misunderstanding it
than I care to admit right now.
PR-URL: https://github.com/nodejs/node/pull/12910
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
If an asynchronous function is passed no callback function, there is no
way to return the result. This patch throws an error if the callback
passed is not valid or none passed at all.
PR-URL: https://github.com/nodejs/node/pull/12562
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Enablie a lint rule to require `===` and `!==` instead of `==` and `!=`
except in some well-defined cases:
* comparing against `null` as a shorthand for also checking for
`undefined`
* comparing the result of `typeof`
* comparing literal values
In cases where `==` or `!=` are being used as optimizations, use an
ESLint comment to disable the `eqeqeq` rule for that line explicitly. I
rather like this because it's a signal that the usage is intentional and
not a mistake.
PR-URL: https://github.com/nodejs/node/pull/12446
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Make sure `constants` object and all the nested objects don't inherit
from `Object.prototype` but from `null`.
PR-URL: https://github.com/nodejs/node/pull/10458
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Infinity and NaN are currently considered valid input when generating a
unix time stamp but are defaulted arbitrarly to Date.now()/1000. This
PR removes this behaviour and throw an exception like all the other
invalid input types.
PR-URL: https://github.com/nodejs/node/pull/11919
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Including:
* Skip URL instance check for common (string) cases
* Avoid regexp on non-Windows platforms when parsing the root of a path
* Skip call to `getOptions()` in common case where no `options` is passed
* Avoid `hasOwnProperty()`
PR-URL: https://github.com/nodejs/node/pull/11665
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Including:
* Move async *stat() functions to FillStatsArray() now used by the
sync *stat() functions
* Avoid creating fs.Stats instances for implicit async/sync *stat()
calls used in various fs functions
* Store reference to Float64Array data on C++ side for easier/faster
access, instead of passing from JS to C++ on every async/sync *stat()
call
PR-URL: https://github.com/nodejs/node/pull/11665
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Replacing the path separator-finding regexp with a custom function
results in a measurable improvement in performance.
PR-URL: https://github.com/nodejs/node/pull/10789
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
nullCheck() implicitly converts the argument to string when checking
the value, so this commit avoids any unnecessary additional (Buffer)
conversions to string.
PR-URL: https://github.com/nodejs/node/pull/10789
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/11522
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This API was never intended to be made public and was docs-only
deprecated in Node.js 6.x. This upgrades to a runtime deprecation
for Node.js 8.0.0
PR-URL: https://github.com/nodejs/node/pull/10467
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Changed the logic in fs.ReadStream and fs.WriteStream so that
close always calls the prototype method rather than the internal
event listener.
Fixes: https://github.com/nodejs/node/issues/2950
PR-URL: https://github.com/nodejs/node/pull/11225
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Updates the fs module APIs to allow 'file://' URL objects
to be passed as the path.
For example:
```js
const URL = require('url').URL;
const myURL = new URL('file:///C:/path/to/file');
fs.readFile(myURL, (err, data) => {});
```
On Windows, file: URLs with a hostname convert to UNC paths,
while file: URLs with drive letters convert to local absolute
paths:
```
file://hostname/a/b/c => \\hostname\a\b\c
file:///c:/a/b/c => c:\a\b\c
```
On all other platforms, file: URLs with a hostname are unsupported
and will result in a throw:
```
file://hostname/a/b/c => throw!
file:///a/b/c => /a/b/c
```
The documentation for the fs API is intentionally not updated in
this commit because the URL API is still considered experimental
and is not officially documented *at this time*
Note that file: URLs are *required* by spec to always be absolute
paths from the file system root.
This is a semver-major commit because it changes error handling
on the fs APIs.
PR-URL: https://github.com/nodejs/node/pull/10739
Ref: https://github.com/nodejs/node/issues/10703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Updates the fs module APIs to allow 'file://' URL objects
to be passed as the path.
For example:
```js
const URL = require('url').URL;
const myURL = new URL('file:///C:/path/to/file');
fs.readFile(myURL, (err, data) => {});
```
On Windows, file: URLs with a hostname convert to UNC paths,
while file: URLs with drive letters convert to local absolute
paths:
```
file://hostname/a/b/c => \\hostname\a\b\c
file:///c:/a/b/c => c:\a\b\c
```
On all other platforms, file: URLs with a hostname are unsupported
and will result in a throw:
```
file://hostname/a/b/c => throw!
file:///a/b/c => /a/b/c
```
The documentation for the fs API is intentionally not updated in
this commit because the URL API is still considered experimental
and is not officially documented *at this time*
Note that file: URLs are *required* by spec to always be absolute
paths from the file system root.
This is a semver-major commit because it changes error handling
on the fs APIs.
PR-URL: https://github.com/nodejs/node/pull/10739
Ref: https://github.com/nodejs/node/issues/10703
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Assigns a static identifier code to all runtime and documentation
only deprecations. The identifier code is included in the emitted
DeprecationWarning.
Also adds a deprecations.md to the API docs to provide a central
location where deprecation codes can be referenced and explained.
PR-URL: https://github.com/nodejs/node/pull/10116
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Michal Zasso <targos@protonmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
The third parameter `err` is not used anywhere.
PR-URL: https://github.com/nodejs/node/pull/10862
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It would previously read from the current file position, which can be
non-zero if the `fd` has been read from or written to. This contradicts
the documentation which states that it "reads the entire contents of a
file".
PR-URL: https://github.com/nodejs/node/pull/9699
Fixes: https://github.com/nodejs/node/issues/9671
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
It is a maintenance burden that was removed from the docs in 2010
(c93e0aaf06) and runtime-deprecated in v6.0
(1124de2d76).
PR-URL: https://github.com/nodejs/node/pull/9683
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Allow `fs.read`, `fs.write` and `fs.writeFile` to take
`Uint8Array` arguments.
PR-URL: https://github.com/nodejs/node/pull/10382
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
Extend `fs.realpathSync` to cache the results for paths that are not
symlinks in addition to caching symlink mappings.
PR-URL: https://github.com/nodejs/node/pull/10253
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This line `pool = null;` isn't needed and has
been around since the first iteration of streams.
I can't find a good reason for it to exist, it's
not more readable, nor does it seem to trick the
compiler into any optimizations.
PR-URL: https://github.com/nodejs/node/pull/10260
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Italo A. Casas <me@italoacasas.com>
The fs function copyObject() had two arguments:
source and target. On the first line of the function it
assigned the target variable to:
arguments.length >= 2 ? target : {};
The function copyObject() was not called directly by
any test, but it is called in other fs functions. When it
was called it was only ever called with a single argument,
source. Thus I have removed the target argument and assigned
it to an empty object like it was being assigned to in the
original ternary operator.
PR-URL: https://github.com/nodejs/node/pull/10041
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Even though an Error object is passed to the callback when readFile()
fails due to toString() failing, it is a bit strange to still see
data passed as the second argument. This commit changes that and only
passes the Error object in that case.
PR-URL: https://github.com/nodejs/node/pull/9670
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Move the internally defined symbol `fs.realpathCacheKey` to
the internal fs module, where it’s more appropriate.
The symbol was recently added in c084287a60, but since
`internal/fs` is only available in the v7.x branch, this
needs to be a separate follow-up change.
PR-URL: https://github.com/nodejs/node/pull/8862
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
The `/` character does not need to be escaped when occurring inside a
character class in a regular expression. Remove such instances of
escaping in the code base.
PR-URL: https://github.com/nodejs/node/pull/9485
Reviewed-By: Prince John Wesley <princejohnwesley@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Add support for fs.write(fd, buffer, cb) and fs.write(fd, buffer, offset, cb)
as documented at
https://nodejs.org/api/fs.html#fs_fs_write_fd_data_position_encoding_callback
and equivalently for fs.writeSync
Update docs and code comments to reflect the implementation.
PR-URL: https://github.com/nodejs/node/pull/7856
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Updates the argument names `srcpath` and `dstpath` to match the more
descriptive `existingPath` and `newPath` in the documentation.
PR-URL: https://github.com/nodejs/node/pull/9145
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
In most cases, named functions match the variable or property to which
they are being assigned. That also seems to be the practice in a series
of PRs currently being evaluated that name currently-anonymous
functions.
This change applies that rule to instances in the code base that don't
comply with that practice.
This will be enforceable with a lint rule once we upgrade to ESLint
3.8.0.
PR-URL: https://github.com/nodejs/node/pull/9113
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
As it is, the "options" processing is repeated in all the functions
which need it. That introduces checks which are inconsistent with
other functions and produces slightly different error messages.
This patch moves the basic "options" validation and processing to a
seperate function.
PR-URL: https://github.com/nodejs/node/pull/7165
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Nicu Micleușanu <micnic90@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reintroduce a realpath cache with the same mechanisms which existed
before b488b19eaf
(`fs: optimize realpath using uv_fs_realpath()`), but only for
the synchronous version and with the cache being passed as a
hidden option to make sure it is only used internally.
The cache is hidden from userland applications because it has been
decided that fully reintroducing as part of the public API might stand
in the way of future optimizations.
PR-URL: https://github.com/nodejs/node/pull/8100
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Emits 'stop' event for fs.watchFile on process.nextTick
to fix 'maximum call stack size exceeded' error when
`stop` is called synchronously after listener is attached.
PR-URL: https://github.com/nodejs/node/pull/8524
Fixes: https://github.com/nodejs/node/issues/8421
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Use process.emitWarning() instead of the internal printDeprecationMessage
in order to avoid use of an internal only API.
PR-URL: https://github.com/nodejs/node/pull/8166
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Move the implementation of SyncWriteStream to internal/fs.
PR-URL: https://github.com/nodejs/node/pull/6749
Reviewed-By: Ron Korving <ron@ronkorving.nl>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
This patch issues a deprecation warning, if an asynchronous function
is called without a callback function.
PR-URL: https://github.com/nodejs/node/pull/7897
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Add a property named bytesRead that exposes how many bytes that have
currently been read from the file. This brings consistency with
WriteStream that has bytesWritten and net.Socket which have both
bytesRead and bytesWritten.
Fixes: https://github.com/nodejs/node/issues/#7938
PR-URL: https://github.com/nodejs/node/pull/7942
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This reverts commit 9359de9dd2.
Original Commit Message:
The "fs" module has two functions called `maybeCallback` and
`makeCallback`, as of now.
The `maybeCallback` creates a default function to report errors, if the
parameter passed is not a function object. Basically, if the callback
is omitted in some cases, this function is used to create a default
callback function.
The `makeCallback`, OTOH, creates a default function only if the
parameter passed is `undefined`, and if it is not a function object it
will throw an `Error`.
This patch removes the `maybeCallback` function and makes the callback
function argument mandatory for all the async functions.
PR-URL: https://github.com/nodejs/node/pull/7168
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/7846
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
This reverts commit c86c1eeab5.
original commit message:
This patch
1. moves the basic validation of arguments to `truncate` family
of functions to the JavaScript layer from the C++ layer.
2. makes sure that the File Descriptors are validated strictly.
PR-URL: #2498
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/7950
Reviewed-By: Julien Gilli <jgilli@nodejs.org>
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Minwoo Jung <jmwsoft@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
This patch
1. moves the basic validation of arguments to `truncate` family
of functions to the JavaScript layer from the C++ layer.
2. makes sure that the File Descriptors are validated strictly.
PR-URL: https://github.com/nodejs/node/pull/2498
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The "fs" module has two functions called `maybeCallback` and
`makeCallback`, as of now.
The `maybeCallback` creates a default function to report errors, if the
parameter passed is not a function object. Basically, if the callback
is omitted in some cases, this function is used to create a default
callback function.
The `makeCallback`, OTOH, creates a default function only if the
parameter passed is `undefined`, and if it is not a function object it
will throw an `Error`.
This patch removes the `maybeCallback` function and makes the callback
function argument mandatory for all the async functions.
PR-URL: https://github.com/nodejs/node/pull/7168
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The name 'event' for the argument of the listener in
fs.watch was confusing considering FSWatcher also had
events. This changes the name of the argument to
eventType.
Fixes: https://github.com/nodejs/node/issues/7504
PR-URL: https://github.com/nodejs/node/pull/7506
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
All the callback functions in `fs` module are supposed to be executed
with no context (`this` value should not be a valid object). But
`mkdtemp`'s callback will have the `FSReqWrap` object as the context.
Sample code to reproduce the problem
'use strict';
const fs = require('fs');
fs.mkdtemp('/tmp/abcd', null, function() {
console.log(this);
});
This would print
FSReqWrap { oncomplete: [Function] }
But that should have printed `null` and this patch fixes that.
PR-URL: https://github.com/nodejs/node/pull/7068
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
As it is, `fs.mkdtemp` crashes with a C++ assertion if the callback
function is not passed. This patch uses `maybeCallback` to create one,
if no callback function is passed.
PR-URL: https://github.com/nodejs/node/pull/6828
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
The require('constants') module is currently undocumented and mashes
together unrelated constants. This refactors the require('constants')
in favor of distinct os.constants, fs.constants, and crypto.constants
that are specific to the modules for which they are relevant. The
next step is to document those within the specific modules.
PR-URL: https://github.com/nodejs/node/pull/6534
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Robert Lindstaedt <robert.lindstaedt@gmail.com>
Aligns the functionality of SlowBuffer with the new Buffer
constructor API. Next step is to docs-only deprecate
SlowBuffer.
Replace the internal uses of SlowBuffer with
`Buffer.allocUnsafeSlow(size)`
PR-URL: https://github.com/nodejs/node/pull/5833
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Remove realpath() and realpathSync() cache.
Use the native uv_fs_realpath() which is faster
then the JS implementation by a few orders of magnitude.
PR-URL: https://github.com/nodejs/node/pull/3594
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Johan Bergström <bugs@bergstroem.nu>
This commit improves module loading performance by at least ~25-35%
in the module-loader benchmarks.
Some optimization strategies include:
* Try-finally/try-catch isolation
* Replacing regular expressions with manual parsing
* Avoiding unnecessary string and array creation
* Avoiding constant recompilation of anonymous functions and
function definitions within functions
PR-URL: https://github.com/nodejs/node/pull/5172
Reviewed-By: James M Snell <jasnell@gmail.com>
This makes several changes:
1. Allow path/filename to be passed in as a Buffer on fs methods
2. Add `options.encoding` to fs.readdir, fs.readdirSync, fs.readlink,
fs.readlinkSync and fs.watch.
3. Documentation updates
For 1... it's now possible to do:
```js
fs.open(Buffer('/fs/foo/bar'), 'w+', (err, fd) => { });
```
For 2...
```js
fs.readdir('/fs/foo/bar', {encoding:'hex'}, (err,list) => { });
fs.readdir('/fs/foo/bar', {encoding:'buffer'}, (err, list) => { });
```
encoding can also be passed as a string
```js
fs.readdir('/fs/foo/bar', 'hex', (err,list) => { });
```
The default encoding is set to UTF8 so this addresses the
discrepency that existed previously between fs.readdir and
fs.watch handling filenames differently.
Fixes: https://github.com/nodejs/node/issues/2088
Refs: https://github.com/nodejs/node/issues/3519
PR-URL: https://github.com/nodejs/node/pull/5616
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
This uses libuv's mkdtemp function to provide a way to create a
temporary folder, using a prefix as the path. The prefix is appended
six random characters. The callback function will receive the name
of the folder that was created.
Usage example:
fs.mkdtemp('/tmp/foo-', function(err, folder) {
console.log(folder);
// Prints: /tmp/foo-Tedi42
});
The fs.mkdtempSync version is also provided. Usage example:
console.log(fs.mkdtemp('/tmp/foo-'));
// Prints: tmp/foo-Tedi42
This pull request also includes the relevant documentation changes
and tests.
PR-URL: https://github.com/nodejs/node/pull/5333
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Saúl Ibarra Corretgé <saghul@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Several changes:
* Soft-Deprecate Buffer() constructors
* Add `Buffer.from()`, `Buffer.alloc()`, and `Buffer.allocUnsafe()`
* Add `--zero-fill-buffers` command line option
* Add byteOffset and length to `new Buffer(arrayBuffer)` constructor
* buffer.fill('') previously had no effect, now zero-fills
* Update the docs
PR-URL: https://github.com/nodejs/node/pull/4682
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Flags on fs.open and others can be passed as strings or int.
Previously, if passing anything other than string or int,
the error message would only say that flags must be an int.
PR-URL: https://github.com/nodejs/node/pull/5590
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This is needed to give users a grace period before actually breaking
modules that re-evaluate fs sources from context where internal modules
are not allowed, e.g. older version of graceful-fs module.
To be reverted in Node.js 7.0
Fixes: #5097, see also #1898, #2026, and #4525.
PR-URL: https://github.com/nodejs/node/pull/5102
Reviewed-By: Rod Vagg <rod@vagg.org>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
`fs.read` supports a deprecated string interface version, which is
not documented. It was intended to be deprecated in this commit in 2010
c93e0aaf06
This patch issues a deprecation message saying the usage of this
interface is deprecated.
PR-URL: https://github.com/nodejs/node/pull/4525
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Two variables are declared twice with `var` in the same scope in
`lib/fs.js`. This change refactors the code so the variable is declared
just once.
PR-URL: https://github.com/nodejs/node/pull/4959
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
In a few places the code was refactored to use `maybeCallback` which
always returns a function. Checking for `if (callback)` always returns
true anyway.
PR-URL: https://github.com/nodejs/node/pull/4795
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
With an indentation style of two spaces, it is not possible to indent
multiline variable declarations by four spaces. Instead, the var keyword
is used on every new line.
Use const instead of var where applicable for changed lines.
PR-URL: https://github.com/nodejs/io.js/pull/2286
Reviewed-By: Roman Reiss <me@silverwind.io>
fs.statSync() creates and returns a heavy-weight fs.Stat object whereas
fs.accessSync() simply returns nothing. The return value is ignored,
the call is for its side effect of throwing an ELOOP error in case of
cyclic symbolic links.
PR-URL: https://github.com/nodejs/node/pull/4575
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Add support to fs.createWriteStream and fs.createWriteStream for an autoClose
option that behaves similarly to the autoClose option supported by
fs.createReadStream and fs.ReadStream.
When an instance of fs.createWriteStream created with autoClose === false finishes,
it is not destroyed. Its underlying fd is not closed and it is the
responsibility of the user to close it.
PR-URL: https://github.com/nodejs/node/pull/3679
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
use `arrow functions` instead of `bind(this)` in order to improve
performance through optimizations.
PR-URL: https://github.com/nodejs/node/pull/3622
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
This commit ensures that readFile() callsback with a null
error consistently on success.
PR-URL: https://github.com/nodejs/node/pull/3740
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
This commit fixes some error messages that are not consistent with
some general rules which most of the error messages follow.
PR-URL: https://github.com/nodejs/node/pull/3374
Reviewed-By: Roman Reiss <me@silverwind.io>
If the resulting buffer.toString() call in fs.read throws, catch the
error and pass it back in the callback.
This issue only presents itself when fs.read is called using the legacy
string interface:
fs.read(fd, length, position, encoding, callback)
PR-URL: https://github.com/nodejs/node/pull/3503
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
These changes affect the following functions and their synchronous
counterparts:
* fs.readFile()
* fs.writeFile()
* fs.appendFile()
If the first parameter is a uint32, it is treated as a file descriptor.
In all other cases, the original implementation is used to ensure
backwards compatibility. File descriptor ownership is never taken from
the user.
The documentation was adjusted to reflect these API changes. A note was
added to make the user aware of file descriptor ownership and the
conditions under which a file descriptor can be used by each of these
functions.
Tests were extended to test for file descriptor parameters under the
conditions noted in the relevant documentation.
PR-URL: https://github.com/nodejs/node/pull/3163
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The `events` module already exports `EventEmitter` constructor function
So, we don't have to use `events.EventEmitter` to access it.
Refer: https://github.com/nodejs/node/pull/2896
PR-URL: https://github.com/nodejs/node/pull/2921
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Michaël Zasso <mic.besace@gmail.com>
Streams with writev allow many buffers to be pushed to underlying OS
APIs in one batch, in this case improving write throughput by an order
of magnitude. This is especially noticeable when writing many (small)
buffers.
PR-URL: https://github.com/nodejs/node/pull/2167
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
As per the discussion in #734, this patch deprecates the usage of
`EventEmitter.listenerCount` static function in the docs, and introduces
the `listenerCount` function in the prototype of `EventEmitter` itself.
PR-URL: https://github.com/nodejs/node/pull/2349
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Reviewed-By: Brian White <mscdex@mscdex.net>
Starting in V8 v4.3 the maximum array index of a typed array is the same
as the largest Smi supported on a given architecture. To compensate for
these differences export kMaxLength from the buffer module with the
correct size for the given architecture.
PR-URL: https://github.com/nodejs/io.js/pull/2003
Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com>
When the listener was truthy but NOT a function, fs.watchFile would
throw an error through the EventEmitter. This caused a problem because
it would only be thrown after the listener was started, which left the
listener on.
There should be no backwards compatability issues because the error was
always thrown, just in a different manner.
Also adds tests for this and other basic functionality.
PR-URL: https://github.com/nodejs/io.js/pull/2093
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Make SyncWriteStream non-enumerable since it's only for internal use.
PR-URL: https://github.com/nodejs/io.js/pull/1870
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1. Remove a few unnecessary variables to reduce LoC.
2. Remove redundant `var` definitions of variables in same function.
3. Refactor variables which are defined inside a block and used outside
as well.
4. Refactor effect-less code.
5. In `rethrow` function, instead of assigning to `err` and throwing
`err` directly throw `backtrace` object.
6. Reassign 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.
PR-URL: https://github.com/nodejs/io.js/pull/1870
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Remove `inStatWatchers` function and make `statWatchers` a `Map`.
PR-URL: https://github.com/nodejs/io.js/pull/1870
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
`nullCheckCallNT()` function is not necessary, as we can directly pass
`callback` and `er` to `process.nextTick()`.
PR-URL: https://github.com/nodejs/io.js/pull/1870
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
1. Change "Bad arguments" error messages to a more helpful message
"options should either be an object or a string".
2. Make braces consistent.
3. Return meaningful error message from fs_event_wrap's
FSEvent's Start function.
PR-URL: https://github.com/nodejs/io.js/pull/1870
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Add string encoding option for fs.createReadStream and
fs.createWriteStream. and check argument type more strictly
PR-URL: https://github.com/nodejs/io.js/pull/1845
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
It makes no sense to allow people use constants from
`smalloc`, since it will be removed completely eventually.
PR-URL: https://github.com/nodejs/io.js/pull/1822
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit better handles calls to process.binding() in lib/ by
no longer lazy loading the bindings (the load times themselves are
rather miniscule compared to the load time of V8) and never reloading
the bindings (which is 172 times slower than referencing a variable with
the same value).
PR-URL: https://github.com/iojs/io.js/pull/1367
Reviewed-By: Brian White <mscdex@mscdex.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1. writeFileSync bumps position incorrectly, causing it to drift in
iteration three and onwards.
2. Append mode files will get corrupted in the middle if writeFile or
writeFileSync iterates multiple times, unless running on Linux. position
starts out as null so first write is OK, but then position will refer to
a location inside an existing file, corrupting that data. Linux ignores
position for append mode files so it doesn't happen there.
This commit fixes these two related issues by bumping position correctly
and by always using null as the position argument to write/writeSync for
append mode files.
PR-URL: https://github.com/iojs/io.js/pull/1063
Reviewed-By: Bert Belder <bertbelder@gmail.com>
Not including `new` adds a useless frame and removes a potentially
useful frame.
PR-URL: https://github.com/iojs/io.js/pull/1246
Reviewed-By: Petka Antonov <petka_antonov@hotmail.com>
Reviewed-By: Roman Reiss <me@silverwind.io>
Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
Using st_size to read non-regular files can lead to not reading all the
data.
PR-URL: https://github.com/iojs/io.js/pull/1074
Reviewed-By: Bert Belder <bertbelder@gmail.com>
This commit adds proper type checking to makeCallback(). Anything
other than undefined or a function will throw.
PR-URL: https://github.com/iojs/io.js/pull/866
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Vladimir Kurchatkin <vladimir.kurchatkin@gmail.com>
Currently, fs.truncate() silently fails when a file descriptor
is passed as the first argument. This commit changes this
behavior to properly call fs.ftruncate().
PR-URL: https://github.com/joyent/node/pull/9161
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Conflicts:
lib/fs.js
Currently, nullCheck() will attempt to invoke any truthy value
as a function if the path argument contains a null character.
This commit validates that the callback is actually a function
before trying to invoke it. fs.access() was vulnerable to this
bug, as nullCheck() was called prior to type checking its
callback.
PR-URL: https://github.com/iojs/io.js/pull/887
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit improves `readFile` performance by
reducing number of closure allocations and using
`FSReqWrap` directly.
PR-URL: https://github.com/iojs/io.js/pull/718
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Many of the util.is*() methods used to check data types
simply compare against a single value or the result of
typeof. This commit replaces calls to these methods with
equivalent checks. This commit does not touch calls to the
more complex methods (isRegExp(), isDate(), etc.).
Fixes: https://github.com/iojs/io.js/issues/607
PR-URL: https://github.com/iojs/io.js/pull/647
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
These flags were defined as constants, but could be
overwritten when exported from fs. This commit exports
the flags as read only properties of fs.
PR-URL: https://github.com/iojs/io.js/pull/507
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit replaces a number of var statements throughout
the lib code with const statements.
PR-URL: https://github.com/iojs/io.js/pull/541
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Define fs constants using const, as the newer version of
v8 supports it, and appears to be capable of optimizing.
PR-URL: https://github.com/iojs/io.js/pull/522
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The copyright and license notice is already in the LICENSE file. There
is no justifiable reason to also require that it be included in every
file, since the individual files are not individually distributed except
as part of the entire package.
fs.exists() and fs.existsSync() were deprecated in #103. This
commit removes the deprecation message, in order to stay more
in sync with joyent/node for the io.js 1.0.0 release.
Fixes: https://github.com/iojs/io.js/issues/257
PR-URL: https://github.com/iojs/io.js/pull/307
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Bert Belder <bertbelder@gmail.com>
Update docs, comments and code to use ES6 octal literals instead of
decimal + comment.
PR-URL: https://github.com/iojs/io.js/pull/281
Reviewed-by: Bert Belder <bertbelder@gmail.com>
Reviewed-by: Colin Ihrig <cjihrig@gmail.com>
fs.exists() and fs.existsSync() do not follow the typical
error first callback convention. access() and accessSync()
are added as alternatives in this commit.
Fixes: https://github.com/joyent/node/pull/8714
PR-URL: https://github.com/iojs/io.js/pull/114
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Domenic Denicola <domenic@domenicdenicola.com>
Instead of simply creating a new v8::Object to contain the connection
information, instantiate a new instance of a FunctionTemplate. This will
allow future improvements for debugging and performance probes.
Additionally, the "provider" argument in the ReqWrap constructor is no
longer optional.
PR-URL: https://github.com/joyent/node/pull/8110
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
Reviewed-by: Fedor Indutny <fedor@indutny.com>
Reviewed-by: Alexis Campailla <alexis@janeasystems.com>
Reviewed-by: Julien Gilli <julien.gilli@joyent.com>
Turn on strict mode for the files in the lib/ directory. It helps
catch bugs and can have a positive effect on performance.
PR-URL: https://github.com/node-forward/node/pull/64
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Fedor Indutny <fedor@indutny.com>
Currently, if fstat() fails in readFile(), the callback
is invoked without closing the file. This commit closes
the file before calling back.
Closes#7697
Currently, if fstat() fails in readFile(), the callback
is invoked without closing the file. This commit closes
the file before calling back.
Closes#7697
stringToFlags() has fall throughs in a case statement.
However, they are not consistently implemented. This commit adds
consistency.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
Oversight to not pass blksize to fs.Stats on initialization.
Also added a test to make sure the object property has been set. Since
now on Windows both blksize and blocks will simply be set to undefined.
By building the fs.Stats object in JS, which is returned by all fs stat
functions, calls to v8::Object::Set() are removed. This also includes
creating all associated Date objects in JS, rather than using
v8::Date::New(). Both these changes have significant performance gains.
Note that the returned value from fs.stat changes slightly for non-POSIX
systems. Whereas before the stats object would be missing blocks and
blksize keys, it now has these keys with undefined as the value.
Signed-off-by: Trevor Norris <trev.norris@gmail.com>
Even if stdio streams are opened as file streams, we should not ever try
to close them. This could be accomplished by passing `autoClose: false`
in options on their creation.
Even if stdio streams are opened as file streams, we should not ever try
to close them. This could be accomplished by passing `autoClose: false`
in options on their creation.
Currently fs.watch does not have an option to specify if a directory
should be recursively watched for events across all subdirectories.
Several file watcher APIs support this. FSEvents on OS X > 10.5 is
one example. libuv has added support for FSEvents, but fs.watch had
no way to specify that a recursive watch was required.
fs.watch now has an additional boolean option 'recursive'. When set
to true, and when supported, fs.watch will return notifications for
the entire directory tree hierarchy rooted at the specified path.
fs.truncate() and its synchronous sibling are implemented in terms of
open() + ftruncate(). Unfortunately, it opened the target file with
mode 'w' a.k.a. 'write-only and create or truncate at open'.
The subsequent call to ftruncate() then moved the end-of-file pointer
from zero to the requested offset with the net result of a file that's
neatly truncated at the right offset and filled with zero bytes only.
This bug was introduced in commit 168a5557 but in fairness, before that
commit fs.truncate() worked like fs.ftruncate() so it seems we've never
had a working fs.truncate() until now.
Fixes#6233.
I haven't actually tested this code, but was reading it due to a
post that linked to the code here:
http://dailyjs.com/2013/09/26/libuv/
As I was reading through the code, I noticed a path that can't
be reached.
I didn't strictly follow the contributing guide:
https://github.com/joyent/node/wiki/Contributing
but the change seems safe.
Feel free to close this out. I'm not sure if it was just an oversight
or what.
Prior, strings would first be converted to a Buffer before being written
to disk. Now the intermediary step has been removed.
Other changes of note:
* Class member "must_free" was added to req_wrap so to track if the
memory needs to be manually cleaned up after use.
* External String Resource support, so the memory will be used directly
instead of copying out the data.
* Docs have been updated to reflect that if position is not a number
then it will assume null. Previously it specified the argument must be
null, but that was not how the code worked. An attempt was made to
only support == null, but there were too many tests that assumed !=
number would be enough.
* Docs update show some of the write/writeSync arguments are optional.
Libuv now returns errors directly. Make everything in src/ and lib/
follow suit.
The changes to lib/ are not strictly necessary but they remove the need
for the abominations that are process._errno and node::SetErrno().
All compile time warnings about using deprecated APIs have been
suppressed by updating node's API. Though there are still many function
calls that can accept Isolate, and still need to be updated.
node_isolate had to be added as an extern variable in node.h and
node_object_wrap.h
Also a couple small fixes for Error handling.
Before v8 3.16.6 the error stack message was lazily written when it was
needed, which allowed you to change the message after instantiation.
Then the stack would be written with the new message the first time it
was accessed. Though that has changed. Now it creates the stack message
on instantiation. So setting a different message afterwards won't be
displayed.
This is not a complete fix for the problem. Getting error without any
message isn't very useful.
Commit a804347 makes fs function rethrow errors when the callback is
omitted. While the right thing to do, it's a change from the old v0.8
behavior where such errors were silently ignored.
To give users time to upgrade, temporarily disable that and replace it
with a function that warns once about the deprecated behavior.
Close#5005
Now that highWaterMark increases when there are large reads, this
greatly reduces the number of calls necessary to _read(size), assuming
that _read actually respects the size argument.
Ability to return just the length of listeners for a given type, using
EventEmitter.listenerCount(emitter, event). This will be a lot cheaper
than creating a copy of the listeners array just to check its length.
This makes it so that `stream.push(chunk)` is the only way to signal the
end of reading, removing the confusing disparity between the
callback-style _read method, and the fact that most real-world streams
do not have a 1:1 corollation between the "please give me data" event,
and the actual arrival of a chunk of data.
It is still possible, of course, to implement a `CallbackReadable` on
top of this. Simply provide a method like this as the callback:
function readCallback(er, chunk) {
if (er)
stream.emit('error', er);
else
stream.push(chunk);
}
However, *only* fs streams actually would behave in this way, so it
makes not a lot of sense to make TCP, TLS, HTTP, and all the rest have
to bend into this uncomfortable paradigm.
It seems like a good idea on the face of it, but lowWaterMarks are
actually not useful, and in practice should always be set to zero.
It would be worthwhile for writers if we actually did some kind of
writev() type of thing, but actually this just delays calling write()
and the overhead of doing a bunch of Buffer copies is not worth the
slight benefit of calling write() fewer times.
This increases fs.WriteStream throughput dramatically by removing the
"higher default water marks" for fs.WriteStream.
Also includes a benchmark. Current performance is significantly higher
than v0.8 for strings at all tested levels except size=1. Buffer
performance is still lackluster.
Further improvement in the stream.Writable base class is required, but
this is a start.
Fix an exception that was raised when the WriteStream was closed
immediately after creating it:
TypeError: Cannot read property 'fd' of undefined
at WriteStream.close (fs.js:1537:18)
<snip>
Avoid the TypeError and make sure the file descriptor is closed.
Fixes#4745.
Make `fs.createReadStream({ end: 42 })` work.
Before this commit, it worked only when used like this:
`fs.createReadStream({ start: 0, end: 42 })` - only when `start` was specified
by the caller.
Fixes#4423.
James M Snell
Jamie Davis
Joyee Cheung
陈刚
Jon Moss
Matteo Collina
Weijia Wang
Steven
Mithun Sasidharan
Calvin Metcalf
Suryanarayana Murthy N
Ben Noordhuis
Anatoli Papirovski
Anna Henningsen
Bryan English
r1cebank
Timothy Gu
cjihrig
matzavinos
Rich Trott
Sebastiaan Deckers
Ruben Bridgewater
Nikolai Vavilov
Refael Ackermann
Dan Fabulich
Ebrahim Byagowi
Daniel Pihlstrom
Myles Borins
Sakthipriyan Vairamani (thefourtheye)
Brian White
Luca Maraschi
Jackson Tian
Jeremy Yallop
Francis Gulotta
Ethan Arrowood
Andreas Lind
Kyle E. Mitchell
Claudio Rodriguez
Bartosz Sosnowski
Linus Unnebäck
Myles Borins
Yuval Brik
Florian MARGAINE
Сковорода Никита Андреевич
Benjamin Gruenbaum
Michaël Zasso
Saquib
Feross Aboukhadijeh
Minwoo Jung
yorkie
Zheng Chaoping
micnic
ronkorving
Evan Lucas
Johannes Wüller
charlierudolph
Trevor Norris
Brendan Ashworth
Roman Reiss
Vladimir Kurchatkin
Yosuke Furukawa
Olov Lassus
Mayhem
Santiago Gimeno
Jeremiah Senkpiel
Jonathan Ong
Bruno Jouhier
Alex Kocharin
Jackson Tian
isaacs
Vincent Weevers
Timothy J Fontaine
James Pickard
iamdoron
Fedor Indutny
Nick Simmons
Jeff Switzer
Gil Pedersen
Raymond Feng
Shigeki Ohtsu
Farid Neshat