Merge pull request #490 from squeek502/fs-read-write-offset

fs_{read,write}: make offset argument optional and default it to -1

docs.md

@@ -2234,18 +2234,22 @@ Equivalent to `open(2)`. Access `flags` may be an integer or one of: `"r"`,
 opened in binary mode. Because of this, the `O_BINARY` and `O_TEXT` flags are
 not supported.
 
-### `uv.fs_read(fd, size, offset, [callback])`
+### `uv.fs_read(fd, size, [offset], [callback])`
 
 **Parameters:**
 - `fd`: `integer`
 - `size`: `integer`
-- `offset`: `integer`
+- `offset`: `integer` or `nil`
 - `callback`: `callable` (async version) or `nil` (sync version)
   - `err`: `nil` or `string`
   - `data`: `string` or `nil`
 
 Equivalent to `preadv(2)`. Returns any data. An empty string indicates EOF.
 
+If `offset` is nil or omitted, it will default to `-1`, which indicates 'use and update the current file offset.'
+
+**Note:** When `offset` is >= 0, the current file offset will not be updated by the read.
+
 **Returns (sync version):** `string` or `fail`
 
 **Returns (async version):** `uv_fs_t userdata`
@@ -2264,18 +2268,22 @@ Equivalent to `unlink(2)`.
 
 **Returns (async version):** `uv_fs_t userdata`
 
-### `uv.fs_write(fd, data, offset, [callback])`
+### `uv.fs_write(fd, data, [offset], [callback])`
 
 **Parameters:**
 - `fd`: `integer`
 - `data`: `buffer`
-- `offset`: `integer`
+- `offset`: `integer` or `nil`
 - `callback`: `callable` (async version) or `nil` (sync version)
   - `err`: `nil` or `string`
   - `bytes`: `integer` or `nil`
 
 Equivalent to `pwritev(2)`. Returns the number of bytes written.
 
+If `offset` is nil or omitted, it will default to `-1`, which indicates 'use and update the current file offset.'
+
+**Note:** When `offset` is >= 0, the current file offset will not be updated by the write.
+
 **Returns (sync version):** `integer` or `fail`
 
 **Returns (async version):** `uv_fs_t userdata`

src/fs.c

@@ -450,15 +450,24 @@ static int luv_fs_read(lua_State* L) {
   luv_ctx_t* ctx = luv_context(L);
   uv_file file = luaL_checkinteger(L, 1);
   int64_t len = luaL_checkinteger(L, 2);
-  int64_t offset = luaL_checkinteger(L, 3);
-  uv_buf_t buf;
+  // -1 offset means "the current file offset is used and updated"
+  int64_t offset = -1;
   int ref;
-  uv_fs_t* req;
+  // both offset and callback are optional
+  if (luv_is_callable(L, 3) && lua_isnoneornil(L, 4)) {
+    ref = luv_check_continuation(L, 3);
+  }
+  else {
+    offset = luaL_optinteger(L, 3, offset);
+    ref = luv_check_continuation(L, 4);
+  }
   char* data = (char*)malloc(len);
-  if (!data) return luaL_error(L, "Failure to allocate buffer");
-  buf = uv_buf_init(data, len);
-  ref = luv_check_continuation(L, 4);
-  req = (uv_fs_t*)lua_newuserdata(L, sizeof(*req));
+  if (!data) {
+    luaL_unref(L, LUA_REGISTRYINDEX, ref);
+    return luaL_error(L, "Failure to allocate buffer");
+  }
+  uv_buf_t buf = uv_buf_init(data, len);
+  uv_fs_t* req = (uv_fs_t*)lua_newuserdata(L, sizeof(*req));
   req->data = luv_setup_req(L, ctx, ref);
   // TODO: find out why we can't just use req->ptr for the base
   ((luv_req_t*)req->data)->data = buf.base;
@@ -477,8 +486,17 @@ static int luv_fs_unlink(lua_State* L) {
 static int luv_fs_write(lua_State* L) {
   luv_ctx_t* ctx = luv_context(L);
   uv_file file = luaL_checkinteger(L, 1);
-  int64_t offset = luaL_checkinteger(L, 3);
-  int ref = luv_check_continuation(L, 4);
+  // -1 offset means "the current file offset is used and updated"
+  int64_t offset = -1;
+  int ref;
+  // both offset and callback are optional
+  if (luv_is_callable(L, 3) && lua_isnoneornil(L, 4)) {
+    ref = luv_check_continuation(L, 3);
+  }
+  else {
+    offset = luaL_optinteger(L, 3, offset);
+    ref = luv_check_continuation(L, 4);
+  }
   uv_fs_t* req = (uv_fs_t*)lua_newuserdata(L, sizeof(*req));
   req->data = luv_setup_req(L, ctx, ref);
   size_t count;

tests/test-fs.lua

@@ -10,6 +10,25 @@ return require('lib/tap')(function (test)
     assert(uv.fs_close(fd))
   end)
 
+  test("read a file sync in chunks", function (print, p, expect, uv)
+    local fd = assert(uv.fs_open('README.md', 'r', tonumber('644', 8)))
+    local stat = assert(uv.fs_fstat(fd))
+    local chunks = {}
+    local numchunks = 8
+    local chunksize = math.ceil(stat.size/numchunks)
+    while true do
+      local chunk, err = uv.fs_read(fd, chunksize)
+      assert(not err, err)
+      if #chunk == 0 then
+        break
+      end
+      table.insert(chunks, chunk)
+    end
+    assert(#chunks == numchunks)
+    assert(#table.concat(chunks) == stat.size)
+    assert(uv.fs_close(fd))
+  end)
+
   test("read a file async", function (print, p, expect, uv)
     uv.fs_open('README.md', 'r', tonumber('644', 8), expect(function (err, fd)
       assert(not err, err)
@@ -32,10 +51,13 @@ return require('lib/tap')(function (test)
   test("fs.write", function (print, p, expect, uv)
     local path = "_test_"
     local fd = assert(uv.fs_open(path, "w", 438))
-    uv.fs_write(fd, "Hello World\n", -1)
-    uv.fs_write(fd, {"with\n", "more\n", "lines\n"}, -1)
-    uv.fs_close(fd)
-    uv.fs_unlink(path)
+    -- a mix of async and sync
+    uv.fs_write(fd, "Hello World\n", expect(function(err, n)
+      assert(not err, err)
+      assert(uv.fs_write(fd, {"with\n", "more\n", "lines\n"}))
+      assert(uv.fs_close(fd))
+      assert(uv.fs_unlink(path))
+    end))
   end)
 
   -- collect garbage after uv.fs_write but before the write callback
@@ -47,7 +69,7 @@ return require('lib/tap')(function (test)
     do
       -- the number here gets coerced into a string
       local t = {"with", 600, "lines"}
-      uv.fs_write(fd, t, -1, function()
+      uv.fs_write(fd, t, function()
         local expectedContents = table.concat(t)
         local stat = assert(uv.fs_fstat(fd))
         assert(stat.size == #expectedContents)