Re: read-file

[Paul Eggert]

Simon Josefsson <address@hidden> writes:

> --- read-file.c       16 Jun 2006 21:26:36 +0200      1.1
> +++ read-file.c       17 Jun 2006 18:22:29 +0200      
> @@ -43,6 +43,9 @@
>    if (!buf)
>      return NULL;
>  
> +  if (ferror (stream))
> +    return NULL;
> +
>    while (!feof (stream))
>      {
>        size_t count;

But that would leak memory and would cause errno to contain junk on
failure, even on a POSIX host.

How about the following patch instead?  It fixes a few other minor
glitches I spotted.  I think it addresses Larry Jones's objections as
well, since it causes the code to always invoke 'read'; on hosts
conforming to the C Standard this is fine, and on other hosts where
feof/ferror isn't sticky, the usual convention is to retry the read
anyway, so it's better in that case.  I haven't tested it, though.

2006-06-19  Paul Eggert  <address@hidden>

        * read-file.c (fread_file): Start with buffer allocation of
        0 bytes rather than 1 byte; this simplifies the code.
        Don't invoke feof; it's not needed.  Refactor to avoid duplicate
        code to free buffer and save/restore errno.
        (internal_read_file): Remove unused local.

--- read-file.c.~1.1.~  2006-06-16 12:40:12.000000000 -0700
+++ read-file.c 2006-06-19 01:10:16.000000000 -0700
@@ -36,16 +36,15 @@
 char *
 fread_file (FILE * stream, size_t * length)
 {
-  char *buf = malloc (1);
-  size_t alloc = 1;
+  char *buf = NULL;
+  size_t alloc = 0;
   size_t size = 0;
+  int save_errno;
 
-  if (!buf)
-    return NULL;
-
-  while (!feof (stream))
+  for (;;)
     {
       size_t count;
+      size_t requested;
 
       if (size + BUFSIZ + 1 > alloc)
        {
@@ -58,32 +57,31 @@ fread_file (FILE * stream, size_t * leng
          new_buf = realloc (buf, alloc);
          if (!new_buf)
            {
-             int save_errno = errno;
-             free (buf);
-             errno = save_errno;
-             return NULL;
+             save_errno = errno;
+             break;
            }
 
          buf = new_buf;
        }
 
-      count = fread (buf + size, 1, alloc - size - 1, stream);
+      requested = alloc - size - 1;
+      count = fread (buf + size, 1, requested, stream);
       size += count;
 
-      if (ferror (stream))
+      if (count != requested)
        {
-         int save_errno = errno;
-         free (buf);
-         errno = save_errno;
-         return NULL;
+         save_errno = errno;
+         if (ferror (stream))
+           break;
+         buf[size] = '\0';
+         *length = size;
+         return buf;
        }
     }
 
-  buf[size] = '\0';
-
-  *length = size;
-
-  return buf;
+  free (buf);
+  errno = save_errno;
+  return NULL;
 }
 
 static char *
@@ -92,7 +90,6 @@ internal_read_file (const char *filename
   FILE *stream = fopen (filename, mode);
   char *out;
   int save_errno;
-  int rc;
 
   if (!stream)
     return NULL;