Avoid pathological behavior in OpenBSD's fread.
authorElliott Hughes <enh@google.com>
Tue, 2 Dec 2014 00:13:30 +0000 (16:13 -0800)
committerElliott Hughes <enh@google.com>
Tue, 2 Dec 2014 22:22:02 +0000 (14:22 -0800)
Bug: https://code.google.com/p/android/issues/detail?id=81155
Bug: 18556607
Change-Id: Idc60976b79610e2202cc42dc393dcb4ca6c42e05

libc/Android.mk
libc/stdio/fread.c [moved from libc/upstream-openbsd/lib/libc/stdio/fread.c with 85% similarity]
tests/stdio_test.cpp

index 13fc2975a1ecaa18e50690b6f9a6ef4787394c43..2270dc607355ea26354413116cdb5ad467618efb 100644 (file)
@@ -58,6 +58,7 @@ libc_common_src_files := \
     bionic/sigsetmask.c \
     bionic/system_properties_compat.c \
     stdio/findfp.c \
+    stdio/fread.c \
     stdio/snprintf.c\
     stdio/sprintf.c \
     stdio/stdio_ext.cpp \
@@ -406,7 +407,6 @@ libc_upstream_openbsd_src_files := \
     upstream-openbsd/lib/libc/stdio/fputs.c \
     upstream-openbsd/lib/libc/stdio/fputwc.c \
     upstream-openbsd/lib/libc/stdio/fputws.c \
-    upstream-openbsd/lib/libc/stdio/fread.c \
     upstream-openbsd/lib/libc/stdio/freopen.c \
     upstream-openbsd/lib/libc/stdio/fscanf.c \
     upstream-openbsd/lib/libc/stdio/fseek.c \
similarity index 85%
rename from libc/upstream-openbsd/lib/libc/stdio/fread.c
rename to libc/stdio/fread.c
index 8a592f6d3f12ade27640044d192cbcf181933d76..e052128cc869d70ce1859336b266ea58dd6d5ef2 100644 (file)
@@ -68,7 +68,23 @@ fread(void *buf, size_t size, size_t count, FILE *fp)
                fp->_r = 0;
        total = resid;
        p = buf;
-       while (resid > (r = fp->_r)) {
+
+       // BEGIN android-added
+       // Avoid pathological behavior on unbuffered files. OpenBSD
+       // will loop reading one byte then memcpying one byte!
+       if ((fp->_flags & __SNBF) != 0) {
+               // We know if we're unbuffered that our buffer is empty, so
+               // we can just read directly.
+               while (resid > 0 && (r = (*fp->_read)(fp->_cookie, p, resid)) > 0) {
+                       p += r;
+                       resid -= r;
+               }
+               FUNLOCKFILE(fp);
+               return ((total - resid) / size);
+       }
+       // END android-added
+
+       while (resid > (size_t)(r = fp->_r)) {
                (void)memcpy((void *)p, (void *)fp->_p, (size_t)r);
                fp->_p += r;
                /* fp->_r = 0 ... done in __srefill */
index 6be372c52cf6a59d328a7dfdfea6cb4f807bf31f..854fc7b7b1703765de829c0449c2a1d848b21fb6 100644 (file)
@@ -813,3 +813,34 @@ TEST(stdio, freopen_CLOEXEC) {
 
   fclose(fp);
 }
+
+// https://code.google.com/p/android/issues/detail?id=81155
+// http://b/18556607
+TEST(stdio, fread_unbuffered_pathological_performance) {
+  FILE* fp = fopen("/dev/zero", "r");
+  ASSERT_TRUE(fp != NULL);
+
+  // Make this stream unbuffered.
+  setvbuf(fp, 0, _IONBF, 0);
+
+  char buf[65*1024];
+  memset(buf, 0xff, sizeof(buf));
+
+  time_t t0 = time(NULL);
+  for (size_t i = 0; i < 1024; ++i) {
+    fread(buf, 64*1024, 1, fp);
+  }
+  time_t t1 = time(NULL);
+
+  fclose(fp);
+
+  // 1024 64KiB reads should have been very quick.
+  ASSERT_LE(t1 - t0, 1);
+
+  for (size_t i = 0; i < 64*1024; ++i) {
+    ASSERT_EQ('\0', buf[i]);
+  }
+  for (size_t i = 64*1024; i < 65*1024; ++i) {
+    ASSERT_EQ('\xff', buf[i]);
+  }
+}