Skip to content

Commit 087c461

Browse files
committed
unix: make uv_fs_read() fill all buffers
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]>
1 parent 6e18112 commit 087c461

File tree

6 files changed

+108
-1
lines changed

6 files changed

+108
-1
lines changed

.gitattributes

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
test/fixtures/lorem_ipsum.txt text eol=lf

Makefile.am

+1
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,7 @@ endif # WINNT
117117

118118
EXTRA_DIST = test/fixtures/empty_file \
119119
test/fixtures/load_error.node \
120+
test/fixtures/lorem_ipsum.txt \
120121
include \
121122
docs \
122123
img \

src/unix/fs.c

+49-1
Original file line numberDiff line numberDiff line change
@@ -278,6 +278,54 @@ static ssize_t uv__fs_open(uv_fs_t* req) {
278278
}
279279

280280

281+
static ssize_t uv__fs_preadv(uv_file fd,
282+
uv_buf_t* bufs,
283+
unsigned int nbufs,
284+
off_t off) {
285+
uv_buf_t* buf;
286+
uv_buf_t* end;
287+
ssize_t result;
288+
ssize_t rc;
289+
size_t pos;
290+
291+
assert(nbufs > 0);
292+
293+
result = 0;
294+
pos = 0;
295+
buf = bufs + 0;
296+
end = bufs + nbufs;
297+
298+
for (;;) {
299+
do
300+
rc = pread(fd, buf->base + pos, buf->len - pos, off + result);
301+
while (rc == -1 && errno == EINTR);
302+
303+
if (rc == 0)
304+
break;
305+
306+
if (rc == -1 && result == 0)
307+
return UV__ERR(errno);
308+
309+
if (rc == -1)
310+
break; /* We read some data so return that, ignore the error. */
311+
312+
pos += rc;
313+
result += rc;
314+
315+
if (pos < buf->len)
316+
continue;
317+
318+
pos = 0;
319+
buf += 1;
320+
321+
if (buf == end)
322+
break;
323+
}
324+
325+
return result;
326+
}
327+
328+
281329
static ssize_t uv__fs_read(uv_fs_t* req) {
282330
#if defined(__linux__)
283331
static int no_preadv;
@@ -307,7 +355,7 @@ static ssize_t uv__fs_read(uv_fs_t* req) {
307355
if (no_preadv) retry:
308356
# endif
309357
{
310-
result = pread(req->file, req->bufs[0].base, req->bufs[0].len, req->off);
358+
result = uv__fs_preadv(req->file, req->bufs, req->nbufs, req->off);
311359
}
312360
# if defined(__linux__)
313361
else {

test/fixtures/lorem_ipsum.txt

+1
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
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.

test/test-fs.c

+54
Original file line numberDiff line numberDiff line change
@@ -2721,6 +2721,60 @@ TEST_IMPL(fs_rename_to_existing_file) {
27212721
}
27222722

27232723

2724+
TEST_IMPL(fs_read_bufs) {
2725+
char scratch[768];
2726+
uv_buf_t bufs[4];
2727+
2728+
ASSERT(0 <= uv_fs_open(NULL, &open_req1,
2729+
"test/fixtures/lorem_ipsum.txt",
2730+
O_RDONLY, 0, NULL));
2731+
ASSERT(open_req1.result >= 0);
2732+
uv_fs_req_cleanup(&open_req1);
2733+
2734+
ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
2735+
NULL, 0, 0, NULL));
2736+
ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
2737+
NULL, 1, 0, NULL));
2738+
ASSERT(UV_EINVAL == uv_fs_read(NULL, &read_req, open_req1.result,
2739+
bufs, 0, 0, NULL));
2740+
2741+
bufs[0] = uv_buf_init(scratch + 0, 256);
2742+
bufs[1] = uv_buf_init(scratch + 256, 256);
2743+
bufs[2] = uv_buf_init(scratch + 512, 128);
2744+
bufs[3] = uv_buf_init(scratch + 640, 128);
2745+
2746+
ASSERT(446 == uv_fs_read(NULL,
2747+
&read_req,
2748+
open_req1.result,
2749+
bufs + 0,
2750+
2, /* 2x 256 bytes. */
2751+
0, /* Positional read. */
2752+
NULL));
2753+
ASSERT(read_req.result == 446);
2754+
uv_fs_req_cleanup(&read_req);
2755+
2756+
ASSERT(190 == uv_fs_read(NULL,
2757+
&read_req,
2758+
open_req1.result,
2759+
bufs + 2,
2760+
2, /* 2x 128 bytes. */
2761+
256, /* Positional read. */
2762+
NULL));
2763+
ASSERT(read_req.result == /* 446 - 256 */ 190);
2764+
uv_fs_req_cleanup(&read_req);
2765+
2766+
ASSERT(0 == memcmp(bufs[1].base + 0, bufs[2].base, 128));
2767+
ASSERT(0 == memcmp(bufs[1].base + 128, bufs[3].base, 190 - 128));
2768+
2769+
ASSERT(0 == uv_fs_close(NULL, &close_req, open_req1.result, NULL));
2770+
ASSERT(close_req.result == 0);
2771+
uv_fs_req_cleanup(&close_req);
2772+
2773+
MAKE_VALGRIND_HAPPY();
2774+
return 0;
2775+
}
2776+
2777+
27242778
TEST_IMPL(fs_read_file_eof) {
27252779
#if defined(__CYGWIN__) || defined(__MSYS__)
27262780
RETURN_SKIP("Cygwin pread at EOF may (incorrectly) return data!");

test/test-list.h

+2
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,7 @@ TEST_DECLARE (fs_utime)
326326
TEST_DECLARE (fs_futime)
327327
TEST_DECLARE (fs_file_open_append)
328328
TEST_DECLARE (fs_stat_missing_path)
329+
TEST_DECLARE (fs_read_bufs)
329330
TEST_DECLARE (fs_read_file_eof)
330331
TEST_DECLARE (fs_event_watch_dir)
331332
TEST_DECLARE (fs_event_watch_dir_recursive)
@@ -913,6 +914,7 @@ TASK_LIST_START
913914
TEST_ENTRY (fs_non_symlink_reparse_point)
914915
#endif
915916
TEST_ENTRY (fs_stat_missing_path)
917+
TEST_ENTRY (fs_read_bufs)
916918
TEST_ENTRY (fs_read_file_eof)
917919
TEST_ENTRY (fs_file_open_append)
918920
TEST_ENTRY (fs_event_watch_dir)

0 commit comments

Comments
 (0)