Send inn-workers mailing list submissions to
        [email protected]

To subscribe or unsubscribe via the World Wide Web, visit
        https://lists.isc.org/mailman/listinfo/inn-workers
or, via email, send a message with subject or body 'help' to
        [email protected]

You can reach the person managing the list at
        [email protected]

When replying, please edit your Subject line so it is more specific
than "Re: Contents of inn-workers digest..."


Today's Topics:

   1. Re: Hardening flags (Russ Allbery)
   2. Re: QIO_BUFFERSIZE (Russ Allbery)
   3. Re: QIO_BUFFERSIZE (Russ Allbery)
   4. Re: Discussion about Cancel-Lock support (Russ Allbery)


----------------------------------------------------------------------

Message: 1
Date: Sun, 06 Dec 2020 15:30:27 -0800
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Hardening flags
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8

Julien ?LIE <[email protected]> writes:

> With libperl.a built without -fPIC, linking with -pie fails for innd:

Yeah, that's to be expected.  In order to create position-independent
output (an executable or a library), all objects linked into it, including
static libraries, have to be built position-independent.

> It means that Perl should at least be built with the following flags:
>   ./Configure -des -Accflags=-fPIC
> otherwise, building INN with Perl support fails if PIE is enabled...
> Same thing for the default build of libpython, but not for others like
> libkrb5 or libdb that seem to include -fPIC in their default build
> options.

I think libkrb5 no longer supports static libraries upstream.

> Should we care for that?

My initial feeling is no mostly because I don't expect many users of INN
to be building their own Perl or Python (generally they come with whatever
distribution they're using), and if they do build them, I think most
people will build them shared rather than static (although I admit I don't
know what the default is).

If it turns out that this is more common than we thought, we can always
do something about that later.

-- 
Russ Allbery ([email protected])             <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.


------------------------------

Message: 2
Date: Sun, 06 Dec 2020 16:05:49 -0800
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: QIO_BUFFERSIZE
Message-ID: <[email protected]>
Content-Type: text/plain

Bo Lindbergh <[email protected]> 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 ([email protected])             <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.


------------------------------

Message: 3
Date: Sun, 06 Dec 2020 16:14:10 -0800
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: QIO_BUFFERSIZE
Message-ID: <[email protected]>
Content-Type: text/plain

Russ Allbery <[email protected]> writes:

> 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
>  
>  /*

Oh, right, the problem isn't the overview method itself, it's overchan,
buffchan, and makehistory.  Updated my local copy of the patch.

-- 
Russ Allbery ([email protected])             <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.


------------------------------

Message: 4
Date: Sun, 06 Dec 2020 16:26:00 -0800
From: Russ Allbery <[email protected]>
To: [email protected]
Subject: Re: Discussion about Cancel-Lock support
Message-ID: <[email protected]>
Content-Type: text/plain; charset=utf-8

Julien ?LIE <[email protected]> writes:

>> My inclination would be to support all of the non-MD5 algorithms for
>> verification but only generate SHA-256.  I don't think there's much
>> gained by using the other algorithms.

>> It looks like Gnus still only supports SHA-1.

> For interoperability reasons, it seems that we'll have to handle a
> transition period, and generate two hashes for each message (SHA-1 and
> SHA-256).  And support all of the non-MD5 algorithms to verify cancel
> keys.

On the generate side, I think it only matters what other servers support,
and I have no feel for what server support is out there and thus whether
we need a transition period for that part.  (What we generate won't matter
for Gnus, since it won't have the secret keys for those hashes anyway.)

Agreed on the verification side.

-- 
Russ Allbery ([email protected])             <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.


------------------------------

Subject: Digest Footer

_______________________________________________
inn-workers mailing list
[email protected]
https://lists.isc.org/mailman/listinfo/inn-workers


------------------------------

End of inn-workers Digest, Vol 126, Issue 4
*******************************************

Reply via email to