Don't fail to read a file because it's non-seekable (e.g., a pipe).

* src/util.c (fread_file_lim): New function.
(__virFileReadAll): Use fread_file_lim, rather than requiring
that stat.st_size provide a usable file size.
* tests/read-non-seekable: New test, for the above.
* tests/Makefile.am (test_scripts): Add read-non-seekable.
* tests/test-lib.sh (mkfifo_or_skip_): New helper function.

ChangeLog

+Mon Apr  8 17:32:07 CET 2008 Jim Meyering <meyering@redhat.com>
+
+	Don't fail to read a file because it's non-seekable (e.g., a pipe).
+	* src/util.c (fread_file_lim): New function.
+	(__virFileReadAll): Use fread_file_lim, rather than requiring
+	that stat.st_size provide a usable file size.
+	* tests/read-non-seekable: New test, for the above.
+	* tests/Makefile.am (test_scripts): Add read-non-seekable.
+	* tests/test-lib.sh (mkfifo_or_skip_): New helper function.
+
 Tue Apr  8 13:24:00 BST 2008 Richard W.M. Jones <rjones@redhat.com>
 
 	* src/qemu_driver.c: Handle errors from fork(2) and pipe(2)

src/util.c

@@ -49,6 +49,10 @@
 
 #include "util-lib.c"
 
+#ifndef MIN
+# define MIN(a, b) ((a) < (b) ? (a) : (b))
+#endif
+
 #define MAX_ERROR_LEN   1024
 
 #define TOLOWER(Ch) (isupper (Ch) ? tolower (Ch) : (Ch))
@@ -283,14 +287,64 @@ virExecNonBlock(virConnectPtr conn,
 
 #endif /* __MINGW32__ */
 
+/* Like gnulib's fread_file, but read no more than the specified maximum
+   number of bytes.  If the length of the input is <= max_len, and
+   upon error while reading that data, it works just like fread_file.  */
+static char *
+fread_file_lim (FILE *stream, size_t max_len, size_t *length)
+{
+    char *buf = NULL;
+    size_t alloc = 0;
+    size_t size = 0;
+    int save_errno;
+
+    for (;;) {
+        size_t count;
+        size_t requested;
+
+        if (size + BUFSIZ + 1 > alloc) {
+            char *new_buf;
+
+            alloc += alloc / 2;
+            if (alloc < size + BUFSIZ + 1)
+                alloc = size + BUFSIZ + 1;
+
+            new_buf = realloc (buf, alloc);
+            if (!new_buf) {
+                save_errno = errno;
+                break;
+            }
+
+            buf = new_buf;
+        }
+
+        /* Ensure that (size + requested <= max_len); */
+        requested = MIN (size < max_len ? max_len - size : 0,
+                         alloc - size - 1);
+        count = fread (buf + size, 1, requested, stream);
+        size += count;
+
+        if (count != requested || requested == 0) {
+            save_errno = errno;
+            if (ferror (stream))
+                break;
+            buf[size] = '\0';
+            *length = size;
+            return buf;
+        }
+    }
+
+    free (buf);
+    errno = save_errno;
+    return NULL;
+}
 
-int __virFileReadAll(const char *path,
-                     int maxlen,
-                     char **buf)
+int __virFileReadAll(const char *path, int maxlen, char **buf)
 {
     FILE *fh;
-    struct stat st;
     int ret = -1;
+    size_t len;
+    char *s;
 
     if (!(fh = fopen(path, "r"))) {
         virLog("Failed to open file '%s': %s",
@@ -298,39 +352,21 @@ int __virFileReadAll(const char *path,
         goto error;
     }
 
-    if (fstat(fileno(fh), &st) < 0) {
-        virLog("Failed to stat file '%s': %s",
-               path, strerror(errno));
+    s = fread_file_lim(fh, maxlen+1, &len);
+    if (s == NULL) {
+        virLog("Failed to read '%s': %s", path, strerror (errno));
         goto error;
     }
 
-    if (S_ISDIR(st.st_mode)) {
-        virLog("Ignoring directory '%s'", path);
+    if (len > maxlen || (int)len != len) {
+        free(s);
+        virLog("File '%s' is too large %d, max %d",
+               path, (int)len, maxlen);
         goto error;
     }
 
-    if (st.st_size > maxlen) {
-        virLog("File '%s' is too large %d, max %d", path, (int)st.st_size, maxlen);
-        goto error;
-    }
-
-    *buf = malloc(st.st_size + 1);
-    if (*buf == NULL) {
-        virLog("Failed to allocate data");
-        goto error;
-    }
-
-    if ((ret = fread(*buf, st.st_size, 1, fh)) != 1) {
-        free(*buf);
-        *buf = NULL;
-        virLog("Failed to read config file '%s': %s",
-               path, strerror(errno));
-        goto error;
-    }
-
-    (*buf)[st.st_size] = '\0';
-
-    ret = st.st_size;
+    *buf = s;
+    ret = len;
 
  error:
     if (fh)
@@ -739,6 +775,5 @@ virParseMacAddr(const char* str, unsigned char *addr)
  *  indent-tabs-mode: nil
  *  c-indent-level: 4
  *  c-basic-offset: 4
- *  tab-width: 4
  * End:
  */

tests/Makefile.am

@@ -46,6 +46,7 @@ noinst_PROGRAMS = xmlrpctest xml2sexprtest sexpr2xmltest virshtest conftest \
 test_scripts = \
 	daemon-conf \
 	int-overflow \
+	read-non-seekable \
 	vcpupin
 
 EXTRA_DIST += $(test_scripts)

tests/read-non-seekable

+#!/bin/sh
+# ensure that certain file-reading commands can handle non-seekable files
+
+# Copyright (C) 2008 Free Software Foundation, Inc.
+
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+if test "$VERBOSE" = yes; then
+  set -x
+  virsh --version
+fi
+
+. $srcdir/test-lib.sh
+
+fail=0
+
+cat <<\EOF > dom
+<domain type='test' id='2'>
+  <name>t2</name>
+  <uuid>004b96e1-2d78-c30f-5aa5-000000000000</uuid>
+  <memory>8388608</memory>
+  <vcpu>2</vcpu>
+  <on_reboot>restart</on_reboot>
+  <on_poweroff>destroy</on_poweroff>
+  <on_crash>restart</on_crash>
+</domain>
+EOF
+
+virsh -c test:///default define dom > /dev/null || fail=1
+
+mkfifo_or_skip_ fifo
+cat dom > fifo &
+
+virsh -c test:///default define fifo > /dev/null || fail=1
+
+(exit $fail); exit $fail

tests/test-lib.sh

@@ -120,6 +120,18 @@ skip_if_root_() { uid_is_privileged_ && skip_test_ "must be run as non-root"; }
 error_() { echo "$0: $@" 1>&2; (exit 1); exit 1; }
 framework_failure() { error_ 'failure in testing framework'; }
 
+mkfifo_or_skip_()
+{
+  test $# = 1 || framework_failure
+  if ! mkfifo "$1"; then
+    # Make an exception of this case -- usually we interpret framework-creation
+    # failure as a test failure.  However, in this case, when running on a SunOS
+    # system using a disk NFS mounted from OpenBSD, the above fails like this:
+    # mkfifo: cannot make fifo `fifo-10558': Not owner
+    skip_test_ 'NOTICE: unable to create test prerequisites'
+  fi
+}
+
 test_dir_=$(pwd)
 
 this_test_() { echo "./$0" | sed 's,.*/,,'; }