fileio: fix read_full_stream() bugs (#3887)

read_full_stream() _always_ allocated twice the memory needed, due to
only breaking the realloc() && fread() loop when fread() returned 0,
requiring another iteration and exponentially enlarged buffer just to
discover the EOF condition.

This also caused file sizes >2MiB && <= 4MiB to erroneously be treated
as E2BIG, due to the inappropriately doubled buffer size exceeding
4*1024*1024.

Also made the 4*1024*1024 magic number a READ_FULL_BYTES_MAX constant.

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index e377a9e00a7e60f18346dbd8c3e01ad28ac7a117..16a6d02872eb7986994bb673fccce468a9de5dc5 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -47,6 +47,8 @@
 #include "umask-util.h"
 #include "utf8.h"
 
+#define READ_FULL_BYTES_MAX (4U*1024U*1024U)
+
 int write_string_stream(FILE *f, const char *line, bool enforce_newline) {
 
         assert(f);
@@ -230,7 +232,7 @@ int read_full_stream(FILE *f, char **contents, size_t *size) {
         if (S_ISREG(st.st_mode)) {
 
                 /* Safety check */
-                if (st.st_size > 4*1024*1024)
+                if (st.st_size > READ_FULL_BYTES_MAX)
                         return -E2BIG;
 
                 /* Start with the right file size, but be prepared for
@@ -245,26 +247,31 @@ int read_full_stream(FILE *f, char **contents, size_t *size) {
                 char *t;
                 size_t k;
 
-                t = realloc(buf, n+1);
+                t = realloc(buf, n + 1);
                 if (!t)
                         return -ENOMEM;
 
                 buf = t;
                 k = fread(buf + l, 1, n - l, f);
+                if (k > 0)
+                        l += k;
 
-                if (k <= 0) {
-                        if (ferror(f))
-                                return -errno;
+                if (ferror(f))
+                        return -errno;
 
+                if (feof(f))
                         break;
-                }
 
-                l += k;
-                n *= 2;
+                /* We aren't expecting fread() to return a short read outside
+                 * of (error && eof), assert buffer is full and enlarge buffer.
+                 */
+                assert(l == n);
 
                 /* Safety check */
-                if (n > 4*1024*1024)
+                if (n >= READ_FULL_BYTES_MAX)
                         return -E2BIG;
+
+                n = MAX(n * 2, READ_FULL_BYTES_MAX);
         }
 
         buf[l] = 0;