Index | Thread | Search

From:
Todd C. Miller <millert@openbsd.org>
Subject:
Re: BUFSIZ-related pessimization in fvwrite.c
To:
enh <enh@google.com>
Cc:
OpenBSD Tech <tech@openbsd.org>
Date:
Fri, 26 Apr 2024 19:06:23 -0600

Download raw body.

Thread
On Fri, 26 Apr 2024 17:05:51 -0600, Todd C. Miller wrote:

> I see no reason not to do this, short writes are handled gracefully.
> I'm not aware of any hard requirement to do buffered writes in
> multiples of the buffer size.  This may just be stdio trying to do
> writes in a multiple of the optimal I/O block size (st_blksize).

Here's a diff relative to our fvwrite.c.

 - todd

Index: lib/libc/stdio/fvwrite.c
===================================================================
RCS file: /cvs/src/lib/libc/stdio/fvwrite.c,v
diff -u -p -u -r1.21 fvwrite.c
--- lib/libc/stdio/fvwrite.c	6 Oct 2023 16:41:02 -0000	1.21
+++ lib/libc/stdio/fvwrite.c	26 Apr 2024 23:11:54 -0000
@@ -31,6 +31,7 @@
  * SUCH DAMAGE.
  */
 
+#include <limits.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
@@ -76,11 +77,12 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 	}
 	if (fp->_flags & __SNBF) {
 		/*
-		 * Unbuffered: write up to BUFSIZ bytes at a time.
+		 * Unbuffered: write up to INT_MAX bytes at a time, to not
+		 * truncate the value of len if it is greater than 2^31 bytes.
 		 */
 		do {
 			GETIOV(;);
-			w = (*fp->_write)(fp->_cookie, p, MIN(len, BUFSIZ));
+			w = (*fp->_write)(fp->_cookie, p, MIN(len, INT_MAX));
 			if (w <= 0)
 				goto err;
 			p += w;
@@ -90,7 +92,8 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 		/*
 		 * Fully buffered: fill partially full buffer, if any,
 		 * and then flush.  If there is no partial buffer, write
-		 * one _bf._size byte chunk directly (without copying).
+		 * entire payload directly (without copying) up to a
+		 * multiple of the buffer size.
 		 *
 		 * String output is a special case: write as many bytes
 		 * as fit, but pretend we wrote everything.  This makes
@@ -134,7 +137,15 @@ __sfvwrite(FILE *fp, struct __suio *uio)
 				if (__sflush(fp))
 					goto err;
 			} else if (len >= (w = fp->_bf._size)) {
-				/* write directly */
+				/*
+				 * Write directly up to INT_MAX or greatest
+				 * multiple of buffer size (whichever is
+				 * smaller), keeping in the memory buffer the
+				 * remaining part of payload that is smaller
+				 * than buffer size.
+				 */
+				if (w != 0)
+					w = MIN(w * (len / w), INT_MAX);
 				w = (*fp->_write)(fp->_cookie, p, w);
 				if (w <= 0)
 					goto err;