@justinc1 discovered that strangely, our fread() implementation fails to read from a block device. This is because even if we fread() 1024 bytes, the implementation only asks from the readv() system call 1023 bytes, and then the last byte. The block device then refuses to make the unaligned read, crashing with an assertion failure.
Turns out that, Musl did this strange thing to work around a bug in Linux's terminal IO, which is explained in: http://git.musl-libc.org/cgit/musl/commit/?id=2cff36a84f268c09f4c9dc5a1340652c8e298dc0 OSv does not have this bug - LineDiscipline::read() only blocks once for the entire readv() scattered buffer, as expected. So we don't need this workaround, which can only hurt performance, and in the present case, prevent fread() from working on a block device. Fixes #877 Signed-off-by: Nadav Har'El <n...@scylladb.com> --- tests/tst-fread.cc | 75 +++++++++++++++++++++++++++++++++++++++++++++++ libc/stdio/__stdio_read.c | 7 ++--- modules/tests/Makefile | 2 +- 3 files changed, 79 insertions(+), 5 deletions(-) create mode 100644 tests/tst-fread.cc diff --git a/tests/tst-fread.cc b/tests/tst-fread.cc new file mode 100644 index 0000000..2da01f5 --- /dev/null +++ b/tests/tst-fread.cc @@ -0,0 +1,75 @@ +/* + * Copyright (C) 2017 Cloudius Systems, Ltd. + * + * This work is open source software, licensed under the terms of the + * BSD license as described in the LICENSE file in the top-level directory. + */ +// To compile on Linux, use: g++ -g -pthread -std=c++11 tests/tst-fread.cc + +#include <stdio.h> +#include <iostream> + + +static int tests = 0, fails = 0; + +template<typename T> +bool do_expect(T actual, T expected, const char *actuals, const char *expecteds, const char *file, int line) +{ + ++tests; + if (actual != expected) { + fails++; + std::cout << "FAIL: " << file << ":" << line << ": For " << actuals + << " expected " << expecteds << "(" << expected << "), saw " + << actual << ".\n"; + return false; + } + std::cout << "OK: " << file << ":" << line << ".\n"; + return true; +} +template<typename T> +bool do_expectge(T actual, T expected, const char *actuals, const char *expecteds, const char *file, int line) +{ + ++tests; + if (actual < expected) { + fails++; + std::cout << "FAIL: " << file << ":" << line << ": For " << actuals + << " expected >=" << expecteds << ", saw " << actual << ".\n"; + return false; + } + std::cout << "OK: " << file << ":" << line << ".\n"; + return true; +} +#define expect(actual, expected) do_expect(actual, expected, #actual, #expected, __FILE__, __LINE__) +#define expectge(actual, expected) do_expectge(actual, expected, #actual, #expected, __FILE__, __LINE__) +#define expect_errno(call, experrno) ( \ + do_expect((long)(call), (long)-1, #call, "-1", __FILE__, __LINE__) && \ + do_expect(errno, experrno, #call " errno", #experrno, __FILE__, __LINE__) ) +#define expect_success(var, call) \ + errno = 0; \ + var = call; \ + do_expectge(var, 0, #call, "0", __FILE__, __LINE__); \ + do_expect(errno, 0, #call " errno", "0", __FILE__, __LINE__); + +int main() +{ + // Test that we can read from a block device with fread(), and not + // get problems with unaligned read. +#ifdef __OSV__ + const char *fn = "/dev/vblk0.1"; +#else + const char *fn = "/dev/nvme0n1"; +#endif + std::cerr << "opening " << fn << "\n"; + FILE *fp = fopen(fn, "r"); + expect(!fp, false); + if (fp) { + char buf[4096]; + auto n = fread(buf, 1, 4096, fp); + expect(n, (size_t)4096); + fclose(fp); + } + + std::cout << "SUMMARY: " << tests << " tests, " << fails << " failures\n"; + return fails == 0 ? 0 : 1; + +} diff --git a/libc/stdio/__stdio_read.c b/libc/stdio/__stdio_read.c index 3abf4a6..8fec9c3 100644 --- a/libc/stdio/__stdio_read.c +++ b/libc/stdio/__stdio_read.c @@ -15,7 +15,7 @@ static void cleanup(void *p) size_t __stdio_read(FILE *f, unsigned char *buf, size_t len) { struct iovec iov[2] = { - { .iov_base = buf, .iov_len = len - !!f->buf_size }, + { .iov_base = buf, .iov_len = len }, { .iov_base = f->buf, .iov_len = f->buf_size } }; ssize_t cnt; @@ -32,10 +32,9 @@ size_t __stdio_read(FILE *f, unsigned char *buf, size_t len) f->rpos = f->rend = 0; return cnt; } - if (cnt <= iov[0].iov_len) return cnt; - cnt -= iov[0].iov_len; + if (cnt <= len) return cnt; + cnt -= len; f->rpos = f->buf; f->rend = f->buf + cnt; - if (f->buf_size) buf[len-1] = *f->rpos++; return len; } diff --git a/modules/tests/Makefile b/modules/tests/Makefile index 8dea970..0ac3d63 100644 --- a/modules/tests/Makefile +++ b/modules/tests/Makefile @@ -86,7 +86,7 @@ tests := tst-pthread.so misc-ramdisk.so tst-vblk.so tst-bsd-evh.so \ tst-pthread-setcancelstate.so tst-syscall.so tst-pin.so tst-run.so \ tst-ifaddrs.so tst-pthread-affinity-inherit.so tst-sem-timed-wait.so \ tst-ttyname.so tst-pthread-barrier.so tst-feexcept.so tst-math.so \ - tst-sigaltstack.so + tst-sigaltstack.so tst-fread.so # libstatic-thread-variable.so tst-static-thread-variable.so \ -- 2.9.4 -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to osv-dev+unsubscr...@googlegroups.com. For more options, visit https://groups.google.com/d/optout.
