fs: write strings directly to disk

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.

doc/api/fs.markdown

@@ -373,19 +373,18 @@ to the completion callback.
 
 Synchronous fsync(2).
 
-## fs.write(fd, buffer, offset, length, position, callback)
+## fs.write(fd, buffer, offset, length[, position], callback)
 
 Write `buffer` to the file specified by `fd`.
 
 `offset` and `length` determine the part of the buffer to be written.
 
 `position` refers to the offset from the beginning of the file where this data
-should be written. If `position` is `null`, the data will be written at the
-current position.
-See pwrite(2).
+should be written. If `typeof position !== 'number'`, the data will be written
+at the current position. See pwrite(2).
 
-The callback will be given three arguments `(err, written, buffer)` where `written`
-specifies how many _bytes_ were written from `buffer`.
+The callback will be given three arguments `(err, written, buffer)` where
+`written` specifies how many _bytes_ were written from `buffer`.
 
 Note that it is unsafe to use `fs.write` multiple times on the same file
 without waiting for the callback. For this scenario,
@@ -395,9 +394,39 @@ On Linux, positional writes don't work when the file is opened in append mode.
 The kernel ignores the position argument and always appends the data to
 the end of the file.
 
-## fs.writeSync(fd, buffer, offset, length, position)
+## fs.write(fd, data[, position[, encoding]], callback)
 
-Synchronous version of `fs.write()`. Returns the number of bytes written.
+Write `data` to the file specified by `fd`.  If `data` is not a Buffer instance
+then the value will be coerced to a string.
+
+`position` refers to the offset from the beginning of the file where this data
+should be written. If `typeof position !== 'number'` the data will be written at
+the current position. See pwrite(2).
+
+`encoding` is the expected string encoding.
+
+The callback will receive the arguments `(err, written, string)` where `written`
+specifies how many _bytes_ the passed string required to be written. Note that
+bytes written is not the same as string characters. See
+[Buffer.byteLength](buffer.html#buffer_class_method_buffer_bytelength_string_encoding).
+
+Unlike when writing `buffer`, the entire string must be written. No substring
+may be specified. This is because the byte offset of the resulting data may not
+be the same as the string offset.
+
+Note that it is unsafe to use `fs.write` multiple times on the same file
+without waiting for the callback. For this scenario,
+`fs.createWriteStream` is strongly recommended.
+
+On Linux, positional writes don't work when the file is opened in append mode.
+The kernel ignores the position argument and always appends the data to
+the end of the file.
+
+## fs.writeSync(fd, buffer, offset, length[, position])
+
+## fs.writeSync(fd, data[, position[, encoding]])
+
+Synchronous versions of `fs.write()`. Returns the number of bytes written.
 
 ## fs.read(fd, buffer, offset, length, position, callback)
 

lib/fs.js

@@ -462,50 +462,59 @@ fs.readSync = function(fd, buffer, offset, length, position) {
   return [str, r];
 };
 
+// usage:
+//  fs.write(fd, buffer, offset, length[, position], callback);
+// OR
+//  fs.write(fd, string[, position[, encoding]], callback);
 fs.write = function(fd, buffer, offset, length, position, callback) {
-  if (!IS_BUFFER(buffer)) {
-    // legacy string interface (fd, data, position, encoding, callback)
-    callback = arguments[4];
-    position = arguments[2];
-    assertEncoding(arguments[3]);
-
-    buffer = new Buffer('' + arguments[1], arguments[3]);
-    offset = 0;
-    length = buffer.length;
+  if (IS_BUFFER(buffer)) {
+    // if no position is passed then assume null
+    if (IS_FUNCTION(position)) {
+      callback = position;
+      position = null;
+    }
+    callback = maybeCallback(callback);
+    var wrapper = function(err, written) {
+      // Retain a reference to buffer so that it can't be GC'ed too soon.
+      callback(err, written || 0, buffer);
+    };
+    return binding.writeBuffer(fd, buffer, offset, length, position, wrapper);
   }
 
-  if (!length) {
-    if (IS_FUNCTION(callback)) {
-      process.nextTick(function() {
-        callback(undefined, 0);
-      });
+  if (IS_STRING(buffer))
+    buffer += '';
+  if (!IS_FUNCTION(position)) {
+    if (IS_FUNCTION(offset)) {
+      position = offset;
+      offset = null;
+    } else {
+      position = length;
     }
-    return;
+    length = 'utf8';
   }
-
-  callback = maybeCallback(callback);
-
-  function wrapper(err, written) {
-    // Retain a reference to buffer so that it can't be GC'ed too soon.
+  callback = maybeCallback(position);
+  position = function(err, written) {
+    // retain reference to string in case it's external
     callback(err, written || 0, buffer);
-  }
-
-  binding.write(fd, buffer, offset, length, position, wrapper);
+  };
+  return binding.writeString(fd, buffer, offset, length, position);
 };
 
+// usage:
+//  fs.writeSync(fd, buffer, offset, length[, position]);
+// OR
+//  fs.writeSync(fd, string[, position[, encoding]]);
 fs.writeSync = function(fd, buffer, offset, length, position) {
-  if (!IS_BUFFER(buffer)) {
-    // legacy string interface (fd, data, position, encoding)
-    position = arguments[2];
-    assertEncoding(arguments[3]);
-
-    buffer = new Buffer('' + arguments[1], arguments[3]);
-    offset = 0;
-    length = buffer.length;
-  }
-  if (!length) return 0;
-
-  return binding.write(fd, buffer, offset, length, position);
+  if (IS_BUFFER(buffer)) {
+    if (IS_UNDEFINED(position))
+      position = null;
+    return binding.writeBuffer(fd, buffer, offset, length, position);
+  }
+  if (!IS_STRING(buffer))
+    buffer += '';
+  if (IS_UNDEFINED(offset))
+    offset = null;
+  return binding.writeString(fd, buffer, offset, length, position);
 };
 
 fs.rename = function(oldPath, newPath, callback) {

src/node_file.cc

@@ -22,8 +22,10 @@
 #include "node.h"
 #include "node_file.h"
 #include "node_buffer.h"
+#include "node_internals.h"
 #include "node_stat_watcher.h"
 #include "req_wrap.h"
+#include "string_bytes.h"
 
 #include <fcntl.h>
 #include <sys/types.h>
@@ -62,10 +64,12 @@ using v8::Value;
 class FSReqWrap: public ReqWrap<uv_fs_t> {
  public:
   FSReqWrap(const char* syscall)
-    : syscall_(syscall) {
+    : must_free_(false),
+      syscall_(syscall) {
   }
 
   const char* syscall() { return syscall_; }
+  bool must_free_; // request is responsible for free'ing memory oncomplete
 
  private:
   const char* syscall_;
@@ -97,6 +101,10 @@ static void After(uv_fs_t *req) {
   FSReqWrap* req_wrap = (FSReqWrap*) req->data;
   assert(&req_wrap->req_ == req);
 
+  // check if data needs to be cleaned
+  if (req_wrap->must_free_ == true)
+    delete[] static_cast<char*>(req_wrap->data_);
+
   // there is always at least one argument. "error"
   int argc = 1;
 
@@ -485,7 +493,7 @@ static void Rename(const FunctionCallbackInfo<Value>& args) {
   if (len < 2) return TYPE_ERROR("new path required");
   if (!args[0]->IsString()) return TYPE_ERROR("old path must be a string");
   if (!args[1]->IsString()) return TYPE_ERROR("new path must be a string");
-  
+
   String::Utf8Value old_path(args[0]);
   String::Utf8Value new_path(args[1]);
 
@@ -650,56 +658,114 @@ static void Open(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-// bytesWritten = write(fd, data, position, enc, callback)
+
 // Wrapper for write(2).
 //
+// bytesWritten = write(fd, buffer, offset, length, position, callback)
 // 0 fd        integer. file descriptor
 // 1 buffer    the data to write
 // 2 offset    where in the buffer to start from
 // 3 length    how much to write
 // 4 position  if integer, position to write at in the file.
 //             if null, write from the current position
-static void Write(const FunctionCallbackInfo<Value>& args) {
+static void WriteBuffer(const FunctionCallbackInfo<Value>& args) {
   HandleScope scope(node_isolate);
 
-  if (!args[0]->IsInt32()) {
-    return THROW_BAD_ARGS;
-  }
+  assert(args[0]->IsInt32());
+  assert(Buffer::HasInstance(args[1]));
 
   int fd = args[0]->Int32Value();
+  Local<Object> obj = args[1].As<Object>();
+  const char* buf = Buffer::Data(obj);
+  size_t buffer_length = Buffer::Length(obj);
+  size_t off = args[2]->Uint32Value();
+  size_t len = args[3]->Uint32Value();
+  int64_t pos = GET_OFFSET(args[4]);
+  Local<Value> cb = args[5];
 
-  if (!Buffer::HasInstance(args[1])) {
-    return ThrowError("Second argument needs to be a buffer");
-  }
+  if (off > buffer_length)
+    return ThrowRangeError("offset out of bounds");
+  if (len > buffer_length)
+    return ThrowRangeError("length out of bounds");
+  if (off + len < off)
+    return ThrowRangeError("off + len overflow");
+  if (off + len > buffer_length)
+    return ThrowRangeError("off + len > buffer.length");
 
-  Local<Object> buffer_obj = args[1]->ToObject();
-  char *buffer_data = Buffer::Data(buffer_obj);
-  size_t buffer_length = Buffer::Length(buffer_obj);
+  buf += off;
 
-  size_t off = args[2]->Int32Value();
-  if (off >= buffer_length) {
-    return ThrowError("Offset is out of bounds");
+  if (cb->IsFunction()) {
+    ASYNC_CALL(write, cb, fd, buf, len, pos)
+    return;
   }
 
-  ssize_t len = args[3]->Int32Value();
-  if (off + len > buffer_length) {
-    return ThrowError("off + len > buffer.length");
-  }
+  SYNC_CALL(write, NULL, fd, buf, len, pos)
+  args.GetReturnValue().Set(SYNC_RESULT);
+}
 
-  ASSERT_OFFSET(args[4]);
-  int64_t pos = GET_OFFSET(args[4]);
 
-  char * buf = (char*)buffer_data + off;
-  Local<Value> cb = args[5];
+// Wrapper for write(2).
+//
+// bytesWritten = write(fd, string, position, enc, callback)
+// 0 fd        integer. file descriptor
+// 1 string    non-buffer values are converted to strings
+// 2 position  if integer, position to write at in the file.
+//             if null, write from the current position
+// 3 enc       encoding of string
+static void WriteString(const FunctionCallbackInfo<Value>& args) {
+  HandleScope scope(node_isolate);
+
+  if (!args[0]->IsInt32())
+    return ThrowTypeError("First argument must be file descriptor");
+
+  Local<Value> cb;
+  Local<Value> string = args[1];
+  int fd = args[0]->Int32Value();
+  char* buf = NULL;
+  int64_t pos;
+  size_t len;
+  bool must_free_ = false;
+
+  // will assign buf and len if string was external
+  if (!StringBytes::GetExternalParts(string,
+                                     const_cast<const char**>(&buf),
+                                     &len)) {
+    enum encoding enc = ParseEncoding(args[3], UTF8);
+    len = StringBytes::StorageSize(string, enc);
+    buf = new char[len];
+    // StorageSize may return too large a char, so correct the actual length
+    // by the write size
+    len = StringBytes::Write(buf, len, args[1], enc);
+    must_free_ = true;
+  }
+  pos = GET_OFFSET(args[2]);
+  cb = args[4];
 
   if (cb->IsFunction()) {
-    ASYNC_CALL(write, cb, fd, buf, len, pos)
-  } else {
-    SYNC_CALL(write, 0, fd, buf, len, pos)
-    args.GetReturnValue().Set(SYNC_RESULT);
+    FSReqWrap* req_wrap = new FSReqWrap("write");
+    int err = uv_fs_write(uv_default_loop(), &req_wrap->req_,
+        fd, buf, len, pos, After);
+    req_wrap->object()->Set(oncomplete_sym, cb);
+    req_wrap->must_free_ = must_free_;
+    req_wrap->Dispatched();
+    req_wrap->data_ = buf;
+    if (err < 0) {
+      uv_fs_t* req = &req_wrap->req_;
+      req->result = err;
+      req->path = NULL;
+      After(req);
+    }
+    return args.GetReturnValue().Set(req_wrap->persistent());
   }
+
+  SYNC_CALL(write, NULL, fd, buf, len, pos)
+  args.GetReturnValue().Set(SYNC_RESULT);
+
+  if (must_free_)
+    delete[] buf;
 }
 
+
 /*
  * Wrapper for read(2).
  *
@@ -918,7 +984,8 @@ void File::Initialize(Handle<Object> target) {
   NODE_SET_METHOD(target, "symlink", Symlink);
   NODE_SET_METHOD(target, "readlink", ReadLink);
   NODE_SET_METHOD(target, "unlink", Unlink);
-  NODE_SET_METHOD(target, "write", Write);
+  NODE_SET_METHOD(target, "writeBuffer", WriteBuffer);
+  NODE_SET_METHOD(target, "writeString", WriteString);
 
   NODE_SET_METHOD(target, "chmod", Chmod);
   NODE_SET_METHOD(target, "fchmod", FChmod);

src/string_bytes.cc

@@ -239,7 +239,9 @@ bool StringBytes::GetExternalParts(Handle<Value> val,
     return true;
   }
 
-  assert(val->IsString());
+  if (!val->IsString())
+    return false;
+
   Local<String> str = Local<String>::New(val.As<String>());
 
   if (str->IsExternalAscii()) {