Hello Elliott,
On 20/01/15(Tue) 16:15, enh wrote:
> that patch wasn't setting the _flags right on error or eof.
Thanks! Below is a version of your diff with some tweaks to match the
recent changes done in -current.
In your original post you've shown some test numbers. Does it mean that
you have a test program for fread(3)? If so do you think it would fit as
test in OpenBSD's regress framework?
I'd suggest you to check the setup of your mail client, apparently it
eats all the tabs and makes impossible to apply your diffs correctly :)
Index: fread.c
===================================================================
RCS file: /home/ncvs/src/lib/libc/stdio/fread.c,v
retrieving revision 1.13
diff -u -p -r1.13 fread.c
--- fread.c 8 Dec 2014 20:40:53 -0000 1.13
+++ fread.c 21 Jan 2015 10:54:56 -0000
@@ -37,18 +37,19 @@
#include <errno.h>
#include "local.h"
+#define MINIMUM(a, b) (((a) < (b)) ? (a) : (b))
+
#define MUL_NO_OVERFLOW (1UL << (sizeof(size_t) * 4))
size_t
fread(void *buf, size_t size, size_t count, FILE *fp)
{
- size_t resid;
- char *p;
- int r;
+ size_t desired_total;
size_t total;
+ char *dst;
/*
- * Extension: Catch integer overflow
+ * Extension: Catch integer overflow.
*/
if ((size >= MUL_NO_OVERFLOW || count >= MUL_NO_OVERFLOW) &&
size > 0 && SIZE_MAX / size < count) {
@@ -57,47 +58,79 @@ fread(void *buf, size_t size, size_t cou
return (0);
}
+ desired_total = count * size;
+ total = desired_total;
+
/*
* ANSI and SUSv2 require a return value of 0 if size or count are 0.
*/
- if ((resid = count * size) == 0)
+ if (total == 0)
return (0);
+
FLOCKFILE(fp);
_SET_ORIENTATION(fp, -1);
+
+ /* XXX: how can this ever happen?! */
if (fp->_r < 0)
fp->_r = 0;
- total = resid;
- p = buf;
- if ((fp->_flags & __SNBF) != 0) {
+
+ /*
+ * Ensure _bf._size is valid.
+ */
+ if (fp->_bf._base == NULL)
+ __smakebuf(fp);
+
+ dst = buf;
+
+ while (total > 0) {
/*
- * We know if we're unbuffered that our buffer is empty, so
- * we can just read directly. This is much faster than the
- * loop below which will perform a series of one byte reads.
+ * Copy data out of the buffer.
*/
- while (resid > 0 && (r = (*fp->_read)(fp->_cookie, p, resid)) >
0) {
- p += r;
- resid -= r;
- }
- FUNLOCKFILE(fp);
- return ((total - resid) / size);
+ size_t buffered_bytes = MINIMUM(fp->_r, total);
+
+ memcpy(dst, fp->_p, buffered_bytes);
+ fp->_p += buffered_bytes;
+ fp->_r -= buffered_bytes;
+ dst += buffered_bytes;
+ total -= buffered_bytes;
+
+ /*
+ * Are we done?
+ */
+ if (total == 0)
+ goto out;
+
+ /*
+ * Do we have so much more to read that we should
+ * avoid copying it through the buffer?
+ */
+ if (total > (size_t) fp->_bf._size)
+ break;
+
+ /*
+ * Less than a buffer to go, so refill the buffer and
+ * go around the loop again.
+ */
+ if (__srefill(fp))
+ goto out;
}
- while (resid > (r = fp->_r)) {
- (void)memcpy((void *)p, (void *)fp->_p, (size_t)r);
- fp->_p += r;
- /* fp->_r = 0 ... done in __srefill */
- p += r;
- resid -= r;
- if (__srefill(fp)) {
- /* no more input: return partial result */
- FUNLOCKFILE(fp);
- return ((total - resid) / size);
+ /*
+ * Read directly into the caller's buffer.
+ */
+ while (total > 0) {
+ ssize_t bytes_read = (*fp->_read)(fp->_cookie, dst, total);
+
+ if (bytes_read <= 0) {
+ fp->_flags = (fp->_r == 0) ? __SEOF : __SERR;
+ break;
}
+ dst += bytes_read;
+ total -= bytes_read;
}
- (void)memcpy((void *)p, (void *)fp->_p, resid);
- fp->_r -= resid;
- fp->_p += resid;
+
+out:
FUNLOCKFILE(fp);
- return (count);
+ return ((desired_total - total) / size);
}
