Skip to content

Commit

Permalink
unix: make uv_fs_read() fill all buffers
Browse files Browse the repository at this point in the history
The fallback for systems that lack preadv() only filled the first
buffer.

This commit rectifies that to fill all (or at least as many as possible)
buffers.

Fixes: libuv#2332
PR-URL: libuv#2338
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Jameson Nash <[email protected]>
Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
  • Loading branch information
bnoordhuis committed Jun 20, 2019
1 parent 6e18112 commit 087c461
Show file tree
Hide file tree
Showing 6 changed files with 108 additions and 1 deletion.
1 change: 1 addition & 0 deletions .gitattributes
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
test/fixtures/lorem_ipsum.txt text eol=lf
1 change: 1 addition & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ endif # WINNT

EXTRA_DIST = test/fixtures/empty_file \
test/fixtures/load_error.node \
test/fixtures/lorem_ipsum.txt \
include \
docs \
img \
Expand Down
50 changes: 49 additions & 1 deletion src/unix/fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,54 @@ static ssize_t uv__fs_open(uv_fs_t* req) {
}


static ssize_t uv__fs_preadv(uv_file fd,
uv_buf_t* bufs,
unsigned int nbufs,
off_t off) {
uv_buf_t* buf;
uv_buf_t* end;
ssize_t result;
ssize_t rc;
size_t pos;

assert(nbufs > 0);

result = 0;
pos = 0;
buf = bufs + 0;
end = bufs + nbufs;

for (;;) {
do
rc = pread(fd, buf->base + pos, buf->len - pos, off + result);
while (rc == -1 && errno == EINTR);

if (rc == 0)
break;

if (rc == -1 && result == 0)
return UV__ERR(errno);

if (rc == -1)
break; /* We read some data so return that, ignore the error. */

pos += rc;
result += rc;

if (pos < buf->len)
continue;

pos = 0;
buf += 1;

if (buf == end)
break;
}

return result;
}


static ssize_t uv__fs_read(uv_fs_t* req) {
#if defined(__linux__)
static int no_preadv;
Expand Down Expand Up @@ -307,7 +355,7 @@ static ssize_t uv__fs_read(uv_fs_t* req) {
if (no_preadv) retry:
# endif
{
result = pread(req->file, req->bufs[0].base, req->bufs[0].len, req->off);
result = uv__fs_preadv(req->file, req->bufs, req->nbufs, req->off);
}
# if defined(__linux__)
else {
Expand Down
1 change: 1 addition & 0 deletions test/fixtures/lorem_ipsum.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.
54 changes: 54 additions & 0 deletions test/test-fs.c
Original file line number Diff line number Diff line change
Expand Up @@ -2721,6 +2721,60 @@ TEST_IMPL(fs_rename_to_existing_file) {
}


TEST_IMPL(fs_read_bufs) {
char scratch[768];
uv_buf_t bufs[4];

ASSERT(0 <= uv_fs_open(NULL, &open_req1,
"test/fixtures/lorem_ipsum.txt",
O_RDONLY, 0, NULL));
ASSERT(open_req1.result >= 0);
uv_fs_req_cleanup(&open_req1);

ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
NULL, 0, 0, NULL));
ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
NULL, 1, 0, NULL));
ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
bufs, 0, 0, NULL));

bufs[0] = uv_buf_init(scratch + 0, 256);
bufs[1] = uv_buf_init(scratch + 256, 256);
bufs[2] = uv_buf_init(scratch + 512, 128);
bufs[3] = uv_buf_init(scratch + 640, 128);

ASSERT(446 == uv_fs_read(NULL,
&read_req,
open_req1.result,
bufs + 0,
2, /* 2x 256 bytes. */
0, /* Positional read. */
NULL));
ASSERT(read_req.result == 446);
uv_fs_req_cleanup(&read_req);

ASSERT(190 == uv_fs_read(NULL,
&read_req,
open_req1.result,
bufs + 2,
2, /* 2x 128 bytes. */
256, /* Positional read. */
NULL));
ASSERT(read_req.result == /* 446 - 256 */ 190);
uv_fs_req_cleanup(&read_req);

ASSERT(0 == memcmp(bufs[1].base + 0, bufs[2].base, 128));
ASSERT(0 == memcmp(bufs[1].base + 128, bufs[3].base, 190 - 128));

ASSERT(0 == uv_fs_close(NULL, &close_req, open_req1.result, NULL));
ASSERT(close_req.result == 0);
uv_fs_req_cleanup(&close_req);

MAKE_VALGRIND_HAPPY();
return 0;
}


TEST_IMPL(fs_read_file_eof) {
#if defined(__CYGWIN__) || defined(__MSYS__)
RETURN_SKIP("Cygwin pread at EOF may (incorrectly) return data!");
Expand Down
2 changes: 2 additions & 0 deletions test/test-list.h
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ TEST_DECLARE (fs_utime)
TEST_DECLARE (fs_futime)
TEST_DECLARE (fs_file_open_append)
TEST_DECLARE (fs_stat_missing_path)
TEST_DECLARE (fs_read_bufs)
TEST_DECLARE (fs_read_file_eof)
TEST_DECLARE (fs_event_watch_dir)
TEST_DECLARE (fs_event_watch_dir_recursive)
Expand Down Expand Up @@ -913,6 +914,7 @@ TASK_LIST_START
TEST_ENTRY (fs_non_symlink_reparse_point)
#endif
TEST_ENTRY (fs_stat_missing_path)
TEST_ENTRY (fs_read_bufs)
TEST_ENTRY (fs_read_file_eof)
TEST_ENTRY (fs_file_open_append)
TEST_ENTRY (fs_event_watch_dir)
Expand Down

0 comments on commit 087c461

Please sign in to comment.