util/os_file: always use the 'grow' mechanism
authorEric Engestrom <eric.engestrom@intel.com>
Wed, 1 May 2019 10:44:16 +0000 (11:44 +0100)
committerEric Engestrom <eric.engestrom@intel.com>
Thu, 16 May 2019 11:56:25 +0000 (12:56 +0100)
Use fstat() only to pre-allocate a big enough buffer.

This fixes a race where if the file grows between fstat() and read()
we would be missing the end of the file, and if the file slims down
read() would just fail.

Fixes: 316964709e21286c2af5 "util: add os_read_file() helper"
Reported-by: Jason Ekstrand <jason@jlekstrand.net>
Signed-off-by: Eric Engestrom <eric.engestrom@intel.com>
Reviewed-by: Jason Ekstrand <jason@jlekstrand.net>
src/util/os_file.c

index ee34a75a2e0d08de96eb757ed269564e1eb339a2..246fd32fdf64408fe99627ca52729cf4ea9b6857 100644 (file)
@@ -38,11 +38,29 @@ readN(int fd, char *buf, size_t len)
    return total ? total : err;
 }
 
-static char *
-read_grow(int fd)
+char *
+os_read_file(const char *filename)
 {
+   /* Note that this also serves as a slight margin to avoid a 2x grow when
+    * the file is just a few bytes larger when we read it than when we
+    * fstat'ed it.
+    * The string's NULL terminator is also included in here.
+    */
    size_t len = 64;
 
+   int fd = open(filename, O_RDONLY);
+   if (fd == -1) {
+      /* errno set by open() */
+      return NULL;
+   }
+
+   /* Pre-allocate a buffer at least the size of the file if we can read
+    * that information.
+    */
+   struct stat stat;
+   if (fstat(fd, &stat) == 0)
+      len += stat.st_size;
+
    char *buf = malloc(len);
    if (!buf) {
       close(fd);
@@ -77,46 +95,6 @@ read_grow(int fd)
    return buf;
 }
 
-char *
-os_read_file(const char *filename)
-{
-   size_t len = 0;
-
-   int fd = open(filename, O_RDONLY);
-   if (fd == -1) {
-      /* errno set by open() */
-      return NULL;
-   }
-
-   struct stat stat;
-   if (fstat(fd, &stat) == 0)
-      len = stat.st_size;
-
-   if (!len)
-      return read_grow(fd);
-
-   /* add NULL terminator */
-   len++;
-
-   char *buf = malloc(len);
-   if (!buf) {
-      close(fd);
-      errno = -ENOMEM;
-      return NULL;
-   }
-
-   ssize_t read = readN(fd, buf, len - 1);
-
-   close(fd);
-
-   if (read == -1)
-      return NULL;
-
-   buf[read] = '\0';
-
-   return buf;
-}
-
 #else
 
 char *