From 087c461ee9c69faff2641bb2046df30cc1988dd0 Mon Sep 17 00:00:00 2001 From: Ben Noordhuis Date: Thu, 20 Jun 2019 12:05:59 +0200 Subject: [PATCH] unix: make uv_fs_read() fill all buffers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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: https://github.com/libuv/libuv/issues/2332 PR-URL: https://github.com/libuv/libuv/pull/2338 Reviewed-By: Anna Henningsen Reviewed-By: Jameson Nash Reviewed-By: Saúl Ibarra Corretgé --- .gitattributes | 1 + Makefile.am | 1 + src/unix/fs.c | 50 +++++++++++++++++++++++++++++++- test/fixtures/lorem_ipsum.txt | 1 + test/test-fs.c | 54 +++++++++++++++++++++++++++++++++++ test/test-list.h | 2 ++ 6 files changed, 108 insertions(+), 1 deletion(-) create mode 100644 .gitattributes create mode 100644 test/fixtures/lorem_ipsum.txt diff --git a/.gitattributes b/.gitattributes new file mode 100644 index 00000000..89297cb7 --- /dev/null +++ b/.gitattributes @@ -0,0 +1 @@ +test/fixtures/lorem_ipsum.txt text eol=lf diff --git a/Makefile.am b/Makefile.am index ed1a7bcb..d213d8f4 100644 --- a/Makefile.am +++ b/Makefile.am @@ -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 \ diff --git a/src/unix/fs.c b/src/unix/fs.c index 58e7cef1..a532d810 100644 --- a/src/unix/fs.c +++ b/src/unix/fs.c @@ -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; @@ -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 { diff --git a/test/fixtures/lorem_ipsum.txt b/test/fixtures/lorem_ipsum.txt new file mode 100644 index 00000000..1b376877 --- /dev/null +++ b/test/fixtures/lorem_ipsum.txt @@ -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. diff --git a/test/test-fs.c b/test/test-fs.c index 6cc22575..2cf8f287 100644 --- a/test/test-fs.c +++ b/test/test-fs.c @@ -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!"); diff --git a/test/test-list.h b/test/test-list.h index 7825bc52..5c1a1c99 100644 --- a/test/test-list.h +++ b/test/test-list.h @@ -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) @@ -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)