QIO_BUFFERSIZE
Russ Allbery
eagle at eyrie.org
Mon Dec 7 00:05:49 UTC 2020
Bo Lindbergh <2bfjdsla52kztwejndzdstsxl9athp at gmail.com> writes:
> lib/qio.c contains:
>> /* A reasonable default buffer size to use. */
>> #define QIO_BUFFERSIZE 8192
> I don't think this is reasonable any more, because it limits the maximum
> length of an overview line in makehistory, and there are definitely
> artic|es in the wild with longer overview data. Having different overview
> size limits in innd and makehistory is just wrong.
This is supposed to only be a fallback, and the actual buffer size is
determined based on st_blksize of the fstat data structure for a file
(which, at least on the ext3 system I'm currently looking at, is 4KB and
thus even smaller). That means that changing this require more than just
changing that #define, so I'm moderately surprised that changing the
#define did anything on your system. That implies to me that either
st_blksize isn't working on your system or something else weird is going
on. What operating system and underlying file system is this?
Regardless, this code smells of excessive fiddly optimization for amounts
of memory that are going to be trivial on any modern system. I'm inclined
to just do something like the following (ignore the test suite cleanup
parts, included just for the sake of completeness):
Index: configure.ac
===================================================================
--- configure.ac (revision 10434)
+++ configure.ac (working copy)
@@ -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>
Index: include/inn/qio.h
===================================================================
--- include/inn/qio.h (revision 10434)
+++ include/inn/qio.h (working copy)
@@ -12,6 +12,11 @@
#include <inn/defines.h>
+/* This is the maximum line length that can be read by a QIO operation.
+ In the current implementation of tradindexed, it therefore must be larger
+ than the longest overview line INN supports. */
+#define QIO_BUFFERSIZE (32 * 1024)
+
BEGIN_DECLS
/*
Index: lib/qio.c
===================================================================
--- lib/qio.c (revision 10434)
+++ lib/qio.c (working copy)
@@ -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;
Index: tests/lib/qio-t.c
===================================================================
--- tests/lib/qio-t.c (revision 10434)
+++ tests/lib/qio-t.c (working copy)
@@ -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;
}
--
Russ Allbery (eagle at eyrie.org) <https://www.eyrie.org/~eagle/>
Please send questions to the list rather than mailing me directly.
<https://www.eyrie.org/~eagle/faqs/questions.html> explains why.
More information about the inn-workers
mailing list