INN commit: branches/2.6 (5 files)

INN Commit rra at isc.org
Sat Dec 19 06:10:49 UTC 2020


    Date: Friday, December 18, 2020 @ 22:10:49
  Author: eagle
Revision: 10457

Use a fixed buffer size for QIO

The buffer size for QIO is also a limit on the length of an
overview line, and the previous complex approach resulted in a
buffer size of 8KB on nearly all systems, which was too small in
practice.

Stop attempting to be clever and size according to the file 
system block size, which is generally small, and instead use a
fixed and predictable buffer size of 32KB, which shouldn't be a
problem on any modern system.

Modified:
  branches/2.6/configure.ac
  branches/2.6/doc/pod/qio.pod
  branches/2.6/include/inn/qio.h
  branches/2.6/lib/qio.c
  branches/2.6/tests/lib/qio-t.c

-------------------+
 configure.ac      |    1 
 doc/pod/qio.pod   |   17 +++----
 include/inn/qio.h |    5 ++
 lib/qio.c         |   40 -----------------
 tests/lib/qio-t.c |  116 +++++++++++++++++++++++-----------------------------
 5 files changed, 65 insertions(+), 114 deletions(-)

Modified: configure.ac
===================================================================
--- configure.ac	2020-12-19 05:57:21 UTC (rev 10456)
+++ configure.ac	2020-12-19 06:10:49 UTC (rev 10457)
@@ -504,7 +504,6 @@
 
 dnl Checks for structures.
 AC_STRUCT_TIMEZONE
-AC_CHECK_MEMBERS([struct stat.st_blksize])
 AC_CHECK_MEMBERS([struct tm.tm_gmtoff])
 AC_CHECK_MEMBERS([struct sockaddr.sa_len], [], [],
     [#include <sys/types.h>

Modified: doc/pod/qio.pod
===================================================================
--- doc/pod/qio.pod	2020-12-19 05:57:21 UTC (rev 10456)
+++ doc/pod/qio.pod	2020-12-19 06:10:49 UTC (rev 10457)
@@ -39,13 +39,10 @@
 analog to stdio's FILE structure and should be treated as a black box by
 all users of these routines.  Only the above API should be used.
 
-B<QIOopen> opens the given file for reading.  For regular files, if your
-system provides that information and the size is reasonable, QIO will use
-the block size of the underlying file system as its buffer size;
-otherwise, it will default to a buffer of S<8 KB>.  Returns a pointer to use
-for subsequent calls, or NULL on error.  B<QIOfdopen> performs the same
-operation except on an already-open file descriptor (I<fd> must designate
-a file open for reading).
+B<QIOopen> opens the given file for reading with a buffer size of 32KiB.
+Returns a pointer to use for subsequent calls, or NULL on error.
+B<QIOfdopen> performs the same operation except on an already-open file
+descriptor (I<fd> must designate a file open for reading).
 
 B<QIOclose> closes the open file and releases any resources used by the
 QIOSTATE structure.  The QIOSTATE pointer should not be used again after
@@ -55,9 +52,9 @@
 a pointer to it, with the trailing newline replaced by nul.  The returned
 pointer is a pointer into a buffer in the QIOSTATE object and therefore
 will remain valid until B<QIOclose> is called on that object.  If EOF is
-reached, an error occurs, or if the line is longer than the buffer size,
-NULL is returned instead.  To distinguish between the error cases, use
-B<QIOerror> and B<QIOtoolong>.
+reached, an error occurs, or if the line is longer than the buffer size
+(32KiB), NULL is returned instead.  To distinguish between the error
+cases, use B<QIOerror> and B<QIOtoolong>.
 
 B<QIOfileno> returns the descriptor of the open file.
 

Modified: include/inn/qio.h
===================================================================
--- include/inn/qio.h	2020-12-19 05:57:21 UTC (rev 10456)
+++ include/inn/qio.h	2020-12-19 06:10:49 UTC (rev 10457)
@@ -12,6 +12,11 @@
 
 #include <inn/defines.h>
 
+/* This is the maximum line length that can be read by a QIO operation.  Since
+   QIO is used by some overview manipulation tools, it must therefore be
+   larger than the longest overview line INN supports. */
+#define QIO_BUFFERSIZE  (32 * 1024)
+
 BEGIN_DECLS
 
 /*

Modified: lib/qio.c
===================================================================
--- lib/qio.c	2020-12-19 05:57:21 UTC (rev 10456)
+++ lib/qio.c	2020-12-19 06:10:49 UTC (rev 10457)
@@ -17,46 +17,8 @@
 #include "inn/qio.h"
 #include "inn/libinn.h"
 
-/* A reasonable default buffer size to use. */
-#define QIO_BUFFERSIZE  8192
 
-#ifdef HAVE_STRUCT_STAT_ST_BLKSIZE
-#define NO_STRUCT_STAT_ST_BLKSIZE_UNUSED
-#else
-#define NO_STRUCT_STAT_ST_BLKSIZE_UNUSED UNUSED
-#endif
-
 /*
-**  Given a file descriptor, return a reasonable buffer size to use for that
-**  file.  Uses st_blksize if available and reasonable, QIO_BUFFERSIZE
-**  otherwise.
-*/
-static size_t
-buffer_size(int fd NO_STRUCT_STAT_ST_BLKSIZE_UNUSED)
-{
-    size_t size = QIO_BUFFERSIZE;
-
-#if HAVE_STRUCT_STAT_ST_BLKSIZE
-    struct stat st;
-
-    /* The Solaris 2.6 man page says that st_blksize is not defined for
-       block or character special devices (and could contain garbage), so
-       only use this value for regular files. */
-    if (fstat(fd, &st) == 0 && S_ISREG(st.st_mode)) {
-        size = st.st_blksize;
-        if (size > (4 * QIO_BUFFERSIZE) || size == 0)
-            size = QIO_BUFFERSIZE;
-	else
-	    while(size < QIO_BUFFERSIZE)
-		size += st.st_blksize;
-    }
-#endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */
-
-    return size;
-}
-
-
-/*
 **  Open a quick file from a descriptor.
 */
 QIOSTATE *
@@ -67,7 +29,7 @@
     qp = xmalloc(sizeof(*qp));
     qp->_fd = fd;
     qp->_length = 0;
-    qp->_size = buffer_size(fd);
+    qp->_size = QIO_BUFFERSIZE;
     qp->_buffer = xmalloc(qp->_size);
     qp->_start = qp->_buffer;
     qp->_end = qp->_buffer;

Modified: tests/lib/qio-t.c
===================================================================
--- tests/lib/qio-t.c	2020-12-19 05:57:21 UTC (rev 10456)
+++ tests/lib/qio-t.c	2020-12-19 06:10:49 UTC (rev 10457)
@@ -1,6 +1,8 @@
 /* $Id$ */
 /* Test suite for the Quick I/O library */
 
+#define LIBTEST_NEW_FORMAT 1
+
 #include "config.h"
 #include "clibrary.h"
 #include <errno.h>
@@ -7,18 +9,19 @@
 #include <fcntl.h>
 #include <sys/stat.h>
 
-#include "inn/messages.h"
 #include "inn/qio.h"
 #include "inn/libinn.h"
 #include "tap/basic.h"
 
+
 static void
 output(int fd, const void *data, size_t size)
 {
     if (xwrite(fd, data, size) < 0)
-        sysdie("Can't write to .testout");
+        sysbail("Can't write to .testout");
 }
 
+
 int
 main(void)
 {
@@ -26,14 +29,9 @@
     unsigned char c;
     char *result;
     int i, count, fd;
-    size_t size = 8192;
     QIOSTATE *qio;
     bool success;
 
-#if HAVE_STRUCT_STAT_ST_BLKSIZE
-    struct stat st;
-#endif
-
     for (c = 1, i = 0; i < 255; i++, c++)
         data[i] = c;
     data[9] = ' ';
@@ -43,27 +41,15 @@
     memcpy(out, data, 255);
     out[255] = '\0';
     fd = open(".testout", O_RDWR | O_CREAT | O_TRUNC, 0644);
-    if (fd < 0) sysdie("Can't create .testout");
+    if (fd < 0)
+        sysbail("Can't create .testout");
 
-#if HAVE_STRUCT_STAT_ST_BLKSIZE
-    /* Mostly duplicate the code from qio.c so that we can test with lines
-       exactly as large as the buffer. */
-    if (fstat(fd, &st) == 0 && S_ISREG(st.st_mode)) {
-        size = st.st_blksize;
-        if (size > 4 * 8192)
-            size = 8192;
-	else
-	    while(size < 8192)
-		size += st.st_blksize;
-    }
-#endif /* HAVE_STRUCT_STAT_ST_BLKSIZE */
-
     /* Start with small, equally sized lines exactly equal to the buffer.
        Then a line equal in size to the buffer, then a short line and
        another line equal in size to the buffer, then a half line and lines
        repeated to fill another buffer, then a line that's one character too
        long.  Finally, put out another short line. */
-    count = size / 256;
+    count = QIO_BUFFERSIZE / 256;
     for (i = 0; i < count; i++)
         output(fd, line, 256);
     for (i = 0; i < count - 1; i++)
@@ -83,82 +69,84 @@
     output(fd, line, 256);
     close(fd);
 
-    test_init(36);
+    plan(36);
 
     /* Now make sure we can read all that back correctly. */
     qio = QIOopen(".testout");
-    ok(1, qio != NULL);
-    ok(2, !QIOerror(qio));
-    ok(3, QIOfileno(qio) > 0);
+    ok(qio != NULL, "QIOpen works");
+    ok(!QIOerror(qio), "...and there is no error state");
+    ok(QIOfileno(qio) > 0, "...and QIOfileno returns something valid");
     if (unlink(".testout") < 0)
-        sysdie("Can't unlink .testout");
+        sysbail("Can't unlink .testout");
     for (success = true, i = 0; i < count; i++) {
         result = QIOread(qio);
         success = (success && !QIOerror(qio) && (QIOlength(qio) == 255)
                    && !strcmp(result, (char *) out));
     }
-    ok(4, success);
-    ok(5, QIOtell(qio) == (off_t) size);
+    ok(success, "Able to read the first QIO_BUFFERSIZE of data");
+    is_int(QIOtell(qio), (off_t) QIO_BUFFERSIZE, "QIOtell is correct");
     result = QIOread(qio);
-    if (strlen(result) < size - 1) {
-        ok(6, false);
+    if (strlen(result) < QIO_BUFFERSIZE - 1) {
+        ok(false, "Read largest possible line");
     } else {
         for (success = true, i = 0; i < count - 1; i++)
             success = success && !memcmp(result + i * 256, data, 256);
         success = success && !memcmp(result + i * 256, data, 255);
-        ok(6, success);
+        ok(success, "Read largest possible line");
     }
-    ok(7, QIOtell(qio) == (off_t) (2 * size));
+    is_int(QIOtell(qio), (off_t) (2 * QIO_BUFFERSIZE), "QIOtell is correct");
     result = QIOread(qio);
-    ok(8, !QIOerror(qio));
-    ok(9, QIOlength(qio) == 0);
-    ok(10, *result == 0);
+    ok(!QIOerror(qio), "No error on reading an empty line");
+    is_int(QIOlength(qio), 0, "Length of the line is 0");
+    is_string(result, "", "...and result is an empty string");
     result = QIOread(qio);
-    if (strlen(result) < size - 1) {
-        ok(11, false);
+    if (strlen(result) < QIO_BUFFERSIZE - 1) {
+        ok(false, "Read largest line again");
     } else {
         for (success = true, i = 0; i < count - 1; i++)
             success = success && !memcmp(result + i * 256, data, 256);
         success = success && !memcmp(result + i * 256, data, 255);
-        ok(11, success);
+        ok(success, "Read largest line again");
     }
-    ok(12, QIOtell(qio) == (off_t) (3 * size + 1));
+    is_int(QIOtell(qio), (off_t) (3 * QIO_BUFFERSIZE + 1),
+           "QIOtell is correct");
     result = QIOread(qio);
-    ok(13, !QIOerror(qio));
-    ok(14, QIOlength(qio) == 127);
-    ok(15, strlen(result) == 127);
-    ok(16, !memcmp(result, data, 127));
+    ok(!QIOerror(qio), "No error on a shorter read");
+    is_int(QIOlength(qio), 127, "Length is 127");
+    is_int(strlen(result), 127, "String is 127");
+    ok(!memcmp(result, data, 127), "Data is correct");
     for (success = true, i = 0; i < count; i++) {
         result = QIOread(qio);
         success = (success && !QIOerror(qio) && (QIOlength(qio) == 255)
                    && !strcmp(result, (char *) out));
     }
-    ok(17, success);
-    ok(18, QIOtell(qio) == (off_t) (4 * size + 129));
+    ok(success, "Able to read another batch of lines");
+    is_int(QIOtell(qio), (off_t) (4 * QIO_BUFFERSIZE + 129),
+           "QIOtell is correct");
     result = QIOread(qio);
-    ok(19, !result);
-    ok(20, QIOerror(qio));
-    ok(21, QIOtoolong(qio));
+    ok(!result, "Failed to read too long of line");
+    ok(QIOerror(qio), "Error reported");
+    ok(QIOtoolong(qio), "...and too long flag is set");
     result = QIOread(qio);
-    ok(22, !QIOerror(qio));
-    ok(23, QIOlength(qio) == 255);
-    ok(24, strlen(result) == 255);
-    ok(25, !memcmp(result, line, 255));
+    ok(!QIOerror(qio), "Reading again succeeds");
+    is_int(QIOlength(qio), 255, "...and returns the next block");
+    is_int(strlen(result), 255, "...and length is correct");
+    ok(!memcmp(result, line, 255), "...and data is correct");
     result = QIOread(qio);
-    ok(26, !result);
-    ok(27, !QIOerror(qio));
-    ok(28, QIOrewind(qio) == 0);
-    ok(29, QIOtell(qio) == 0);
+    ok(!result, "End of file reached");
+    ok(!QIOerror(qio), "...with no error");
+    is_int(QIOrewind(qio), 0, "QIOrewind works");
+    is_int(QIOtell(qio), 0, "...and QIOtell is correct");
     result = QIOread(qio);
-    ok(30, !QIOerror(qio));
-    ok(31, QIOlength(qio) == 255);
-    ok(32, strlen(result) == 255);
-    ok(33, !strcmp(result, (char *) out));
-    ok(34, QIOtell(qio) == 256);
+    ok(!QIOerror(qio), "Reading the first line works");
+    is_int(QIOlength(qio), 255, "...and QIOlength is correct");
+    is_int(strlen(result), 255, "...and the length is correct");
+    ok(!strcmp(result, (char *) out), "...and the data is correct");
+    is_int(QIOtell(qio), 256, "...and QIOtell is correct");
     fd = QIOfileno(qio);
     QIOclose(qio);
-    ok(35, close(fd) < 0);
-    ok(36, errno == EBADF);
+    ok(close(fd) < 0, "QIOclose closed the file descriptor");
+    is_int(errno, EBADF, "...as confirmed by errno");
 
     return 0;
 }



More information about the inn-committers mailing list