[PATCH] Rewrite and fix grub_bufio_read()

[Stefan Fritsch]

We had problems downloading large (10-100 MB) gziped files via http
with grub. After some debugging, I think I have found the reason in
grub_bufio_read, which leads to wrong data being passed on. This patch
fixes the issues for me.  I did my tests with a somewhat older
snapshot, but the bufio.c file is the same.

Cheers,
Stefan

The grub_bufio_read() had some issues:

* in the calculation of next_buf, it assumed that bufio->block_size is a
  power of 2, which is not always the case (see code in grub_bufio_open()).

* it assumed that the len passed to grub_bufio_read() is not larger
  than bufio->block_size. I have no idea if this is always true, but it
  seems rather fragile to depend on that.

* depending on the seek patterns, it can do much unbuffered reading

This patch rewrites the function, making it more readable and fixing
these issues. It also loops until the requested amount of bytes has
been read.
---
 grub-core/io/bufio.c | 104 ++++++++++++++++++++-------------------------------
 1 file changed, 40 insertions(+), 64 deletions(-)

diff --git a/grub-core/io/bufio.c b/grub-core/io/bufio.c
index 2243827..2ee96e4 100644
--- a/grub-core/io/bufio.c
+++ b/grub-core/io/bufio.c
@@ -103,81 +103,57 @@ static grub_ssize_t
 grub_bufio_read (grub_file_t file, char *buf, grub_size_t len)
 {
   grub_size_t res = 0;
-  grub_off_t next_buf;
   grub_bufio_t bufio = file->data;
   grub_ssize_t really_read;
 
-  if (file->size == GRUB_FILE_SIZE_UNKNOWN)
-    file->size = bufio->file->size;
-
-  /* First part: use whatever we already have in the buffer.  */
-  if ((file->offset >= bufio->buffer_at) &&
-      (file->offset < bufio->buffer_at + bufio->buffer_len))
+  while (len > 0)
     {
-      grub_size_t n;
-      grub_uint64_t pos;
-
-      pos = file->offset - bufio->buffer_at;
-      n = bufio->buffer_len - pos;
-      if (n > len)
-        n = len;
-
-      grub_memcpy (buf, &bufio->buffer[pos], n);
-      len -= n;
-      res += n;
-
-      buf += n;
-    }
-  if (len == 0)
-    return res;
 
-  /* Need to read some more.  */
-  next_buf = (file->offset + res + len - 1) & ~((grub_off_t) bufio->block_size 
- 1);
-  /* Now read between file->offset + res and bufio->buffer_at.  */
-  if (file->offset + res < next_buf)
-    {
-      grub_size_t read_now;
-      read_now = next_buf - (file->offset + res);
-      grub_file_seek (bufio->file, file->offset + res);
-      really_read = grub_file_read (bufio->file, buf, read_now);
-      if (really_read < 0)
-       return -1;
       if (file->size == GRUB_FILE_SIZE_UNKNOWN)
        file->size = bufio->file->size;
-      len -= really_read;
-      buf += really_read;
-      res += really_read;
 
-      /* Partial read. File ended unexpectedly. Save the last chunk in buffer.
-       */
-      if (really_read != (grub_ssize_t) read_now)
+      /* First part: use whatever we already have in the buffer.  */
+      if ((file->offset + res >= bufio->buffer_at) &&
+         (file->offset + res < bufio->buffer_at + bufio->buffer_len))
        {
-         bufio->buffer_len = really_read;
-         if (bufio->buffer_len > bufio->block_size)
-           bufio->buffer_len = bufio->block_size;
-         bufio->buffer_at = file->offset + res - bufio->buffer_len;
-         grub_memcpy (&bufio->buffer[0], buf - bufio->buffer_len,
-                      bufio->buffer_len);
-         return res;
+         grub_size_t n;
+         grub_uint64_t pos;
+
+         pos = file->offset + res - bufio->buffer_at;
+         n = bufio->buffer_len - pos;
+         if (n > len)
+           n = len;
+
+         grub_memcpy (buf, &bufio->buffer[pos], n);
+         len -= n;
+         res += n;
+
+         buf += n;
        }
-    }
+      if (len == 0)
+       return res;
+
+      /* Need to read some more.  */
+
+      if ((file->offset + res < bufio->buffer_at) ||
+         (file->offset + res >= bufio->buffer_at + bufio->block_size))
+        {
+         /* There is no space left in the buffer, move start of buffer */
+         bufio->buffer_at = file->offset + res;
+         bufio->buffer_len = 0;
+        }
+
+      grub_file_seek (bufio->file, bufio->buffer_at + bufio->buffer_len);
 
-  /* Read into buffer.  */
-  grub_file_seek (bufio->file, next_buf);
-  really_read = grub_file_read (bufio->file, bufio->buffer,
-                               bufio->block_size);
-  if (really_read < 0)
-    return -1;
-  bufio->buffer_at = next_buf;
-  bufio->buffer_len = really_read;
-
-  if (file->size == GRUB_FILE_SIZE_UNKNOWN)
-    file->size = bufio->file->size;
-
-  if (len > bufio->buffer_len)
-    len = bufio->buffer_len;
-  grub_memcpy (buf, &bufio->buffer[file->offset + res - next_buf], len);
-  res += len;
+      really_read = grub_file_read (bufio->file, 
&bufio->buffer[bufio->buffer_len],
+                                   bufio->block_size - bufio->buffer_len);
+
+      if (really_read < 0)
+       return -1;
+      if (really_read == 0)
+       return res;
+      bufio->buffer_len += really_read;
+    }
 
   return res;
 }
-- 
2.1.4