basic/fileio: fix reading of not-too-small virtual files

This code is trying to do two things: when reading a file with working
st.st_size, detect when the file size changes between the fstat() and our
allocation of the buffer based on the returned size, and the subsequent read().
When reading a file without st.st_size, read up to READ_FULL_BYTES_MAX.

But this second scenario was partially broken: we'd start with size = 4095, and
double the size up to three times, i.e. up to 32767. But we want to read up to
READ_FULL_BYTES_MAX.

So let's listentangle the two cases a bit: if a file returns non-zero st._size,
proceed as before. But if we don't know the size, let's immediately allocate
the buffer of maximum size of READ_FULL_BYTES_MAX. I think that allocating 4MB
and 1MB is going to take pretty much the same time as long as the memory is not
written to, so by allocating 1MB, 2MB, and 4MB, we wouldn't really be saving
anything internally, but wasting time on repeated reads, if the file is long
enough.

Also, don't do the seek if we know we're going to return an error immediately
after.

This should fix reading of any files in /proc, which all have size == 0. In
particular, various files read by coredump might be larger than 32767.

What about /sys? The file there return a fake value, usually 4096. So we'll
allocate a small buffer and read that.

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index a158ab080d74d4466c183e7eea9ab8a96a268cea..df30870a1a25f90c76fb2d9d251891958349bc55 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -385,20 +385,10 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
         if (fd < 0)
                 return -errno;
 
-        /* Start size for files in /proc/ which usually report a file size of 0. (Files in /sys/ report a
-         * file size of 4K, which is probably OK for sizing our initial buffer, and sysfs attributes can't be
-         * larger anyway.)
-         *
-         * It's one less than 4k, so that the malloc() below allocates exactly 4k. */
-        size = 4095;
-
         /* Limit the number of attempts to read the number of bytes returned by fstat(). */
         n_retries = 3;
 
         for (;;) {
-                if (n_retries <= 0)
-                        return -EIO;
-
                 if (fstat(fd, &st) < 0)
                         return -errno;
 
@@ -409,24 +399,20 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                 assert_cc(READ_FULL_BYTES_MAX < SSIZE_MAX);
                 if (st.st_size > 0) {
                         if (st.st_size > READ_FULL_BYTES_MAX)
-                                return -E2BIG;
+                                return -EFBIG;
 
                         size = st.st_size;
                         n_retries--;
                 } else {
-                        /* Double the buffer size */
-                        if (size >= READ_FULL_BYTES_MAX)
-                                return -E2BIG;
-                        if (size > READ_FULL_BYTES_MAX / 2 - 1)
-                                size = READ_FULL_BYTES_MAX; /* clamp to max */
-                        else
-                                size = size * 2 + 1; /* Stay always one less than page size, so we malloc evenly */
+                        size = READ_FULL_BYTES_MAX;
+                        n_retries = 0;
                 }
 
                 buf = malloc(size + 1);
                 if (!buf)
                         return -ENOMEM;
-                size = malloc_usable_size(buf) - 1; /* Use a bigger allocation if we got it anyway */
+                /* Use a bigger allocation if we got it anyway, but not more than the limit. */
+                size = MIN(malloc_usable_size(buf) - 1, READ_FULL_BYTES_MAX);
 
                 for (;;) {
                         ssize_t k;
@@ -453,6 +439,9 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                  * processing, let's try again either with a bigger guessed size or the new
                  * file size. */
 
+                if (n_retries <= 0)
+                        return st.st_size > 0 ? -EIO : -EFBIG;
+
                 if (lseek(fd, 0, SEEK_SET) < 0)
                         return -errno;
 
@@ -466,8 +455,7 @@ int read_full_virtual_file(const char *filename, char **ret_contents, size_t *re
                 p = realloc(buf, n + 1);
                 if (!p)
                         return -ENOMEM;
-
-                buf = TAKE_PTR(p);
+                buf = p;
         }
 
         if (ret_size)