[PATCH 5/7] ios: Change from getchar to pread device interface.

[Eric Blake]

Now that we are passing an offset to every I/O operation, it is not
that much harder to switch from an interface that operates on one byte
at a time to one that operates on a buffer at an offset: pread/pwrite.
For the file driver, there is not pread counterpart in stdio, so we
still have to fseeko before fread, but as argued in the previous
patch, we're still probably better off sticking to stdio buffers than
additional syscalls were we to rewrite to use pread/pwrite on fds.

The caller, ios.c, still performs most operations one byte at a time,
but that will be cleaned up in subsequent patches.

* src/ios-dev.h (struct ios_dev_if): Replace get_c/put_c with
pread/pwrite.
* src/ios.c (IOS_GET_C_ERR_CHCK, IOS_PUT_C_ERR_CHCK)
(ios_read_string, ios_write_string): Use the new interface.
* src/ios-dev-file.c (ios_dev_file_getc, ios_dev_file_putc):
Rewrite to...
(ios_dev_file_pread, ios_dev_file_pwrite): ...this.
* src/ios-dev-mem.c (ios_dev_mem_getc, ios_dev_mem_putc): Rewrite
to...
(ios_dev_mem_pread, ios_dev_mem_pwrite): ...this.
---
 ChangeLog          | 14 ++++++++++++++
 src/ios-dev.h      | 21 ++++++++++-----------
 src/ios-dev-file.c | 29 +++++++++++++++++++----------
 src/ios-dev-mem.c  | 27 +++++++++++++++------------
 src/ios.c          | 27 +++++++++++++--------------
 5 files changed, 71 insertions(+), 47 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index c554c5ca..0077f4b7 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2020-02-28  Eric Blake  <address@hidden>
+
+       ios: Change from getchar to pread device interface.
+       * src/ios-dev.h (struct ios_dev_if): Replace get_c/put_c with
+       pread/pwrite.
+       * src/ios.c (IOS_GET_C_ERR_CHCK, IOS_PUT_C_ERR_CHCK)
+       (ios_read_string, ios_write_string): Use the new interface.
+       * src/ios-dev-file.c (ios_dev_file_getc, ios_dev_file_putc):
+       Rewrite to...
+       (ios_dev_file_pread, ios_dev_file_pwrite): ...this.
+       * src/ios-dev-mem.c (ios_dev_mem_getc, ios_dev_mem_putc): Rewrite
+       to...
+       (ios_dev_mem_pread, ios_dev_mem_pwrite): ...this.
+
 2020-02-28  Eric Blake  <address@hidden>

        ios: Drop unused seek/tell callbacks.
diff --git a/src/ios-dev.h b/src/ios-dev.h
index 5cc786cb..fa5f6a06 100644
--- a/src/ios-dev.h
+++ b/src/ios-dev.h
@@ -17,10 +17,9 @@
  */

 /* An IO space operates on one or more "IO devices", which are
-   abstractions providing byte-oriented operations, such as
-   positioning, reading bytes, and writing bytes.  Typical abstracted
-   entities are files stored in some file system, the memory of a
-   process, etc.
+   abstractions providing pread and pwrite byte-oriented operations.
+   Typical abstracted entities are files stored in some file system,
+   the memory of a process, etc.

    Since the IO devices are byte-oriented, aspects like endianness,
    alignment and negative encoding are not of consideration.
@@ -33,7 +32,7 @@ typedef uint64_t ios_dev_off;

 #define IOD_EOF -1

-/* Error codes to be used in the inteface below.  */
+/* Error codes to be used in the interface below.  */

 #define IOD_ERROR  1 /* Generic error.     */
 #define IOD_EINVAL 2 /* Invalid argument.  */
@@ -63,15 +62,15 @@ struct ios_dev_if

   int (*close) (void *dev);

-  /* Read a byte from the given device at the current position.
-     Return the byte in an int, or IOD_EOF on error.  */
+  /* Read a small byte buffer from the given device at the given byte offset.
+     Return 0 on success, or IOD_EOF on error, including on short reads.  */

-  int (*get_c) (void *dev, ios_dev_off offset);
+  int (*pread) (void *dev, void *buf, size_t count, ios_dev_off offset);

-  /* Write a byte to the given device at the current position.  Return
-     the character written as an int, or IOD_EOF on error.  */
+  /* Write a small byte buffer to the given device at the given byte offset.
+     Return 0 on success, or IOD_EOF on error, including short writes.  */

-  int (*put_c) (void *dev, int c, ios_dev_off offset);
+  int (*pwrite) (void *dev, const void *buf, size_t count, ios_dev_off offset);

   /* Return the flags of the device, as it was opened.  */

diff --git a/src/ios-dev-file.c b/src/ios-dev-file.c
index 3174e0b8..6a591194 100644
--- a/src/ios-dev-file.c
+++ b/src/ios-dev-file.c
@@ -143,29 +143,38 @@ ios_dev_file_get_flags (void *iod)


 static int
-ios_dev_file_getc (void *iod, ios_dev_off offset)
+ios_dev_file_pread (void *iod, void *buf, size_t count, ios_dev_off offset)
 {
   struct ios_dev_file *fio = iod;
   int ret;

+  /* We are using FILE* for buffering, rather than low-level fd, so we
+     have to fake low-level pread by using fseeko.  */
   if (fseeko (fio->file, offset, SEEK_SET) == -1)
     return IOD_EOF;
-  ret = fgetc (fio->file);
+  ret = fread (buf, 1, count, fio->file);

-  return ret == EOF ? IOD_EOF : ret;
+  /* XXX As long as count <= 9 because the ios layer reads at most an
+     unaligned uint64_t, we are unlikely to hit short reads.  But if
+     future code adds in large buffer reads, we may want to retry on
+     short reads rather than giving up right away. */
+  return ret == count ? 0 : IOD_EOF;
 }

 static int
-ios_dev_file_putc (void *iod, int c, ios_dev_off offset)
+ios_dev_file_pwrite (void *iod, const void *buf, size_t count,
+                    ios_dev_off offset)
 {
   struct ios_dev_file *fio = iod;
   int ret;

-  if (fseeko (fio->file, offset, SEEK_SET) == -1)
+  /* We are using FILE* for buffering, rather than low-level fd, so we
+     have to fake low-level pread by using fseeko.  */
+  if (fseeko (fio->file, offset, SEEK_SET))
     return IOD_EOF;
-  ret = putc (c, fio->file);
-  //printf ("%d -> 0x%lu\n", ret, ftello (fio->file));
-  return ret == EOF ? IOD_EOF : ret;
+  ret = fwrite (buf, 1, count, fio->file);
+
+  return ret == count ? 0 : IOD_EOF;
 }

 static ios_dev_off
@@ -183,8 +192,8 @@ struct ios_dev_if ios_dev_file =
    .handler_normalize = ios_dev_file_handler_normalize,
    .open = ios_dev_file_open,
    .close = ios_dev_file_close,
-   .get_c = ios_dev_file_getc,
-   .put_c = ios_dev_file_putc,
+   .pread = ios_dev_file_pread,
+   .pwrite = ios_dev_file_pwrite,
    .get_flags = ios_dev_file_get_flags,
    .size = ios_dev_file_size,
   };
diff --git a/src/ios-dev-mem.c b/src/ios-dev-mem.c
index 22938909..ad6f0932 100644
--- a/src/ios-dev-mem.c
+++ b/src/ios-dev-mem.c
@@ -22,6 +22,7 @@
 #include <string.h>
 #include <stdlib.h>
 #include <xalloc.h>
+#include <assert.h>

 #include "ios.h"
 #include "ios-dev.h"
@@ -70,7 +71,7 @@ ios_dev_mem_close (void *iod)
 }

 static uint64_t
-ios_dev_mem_get_flags  (void *iod)
+ios_dev_mem_get_flags (void *iod)
 {
   struct ios_dev_mem *mio = iod;

@@ -78,28 +79,30 @@ ios_dev_mem_get_flags  (void *iod)
 }

 static int
-ios_dev_mem_getc (void *iod, ios_dev_off offset)
+ios_dev_mem_pread (void *iod, void *buf, size_t count, ios_dev_off offset)
 {
   struct ios_dev_mem *mio = iod;

-  if (offset >= mio->size)
+  if (offset + count >= mio->size)
     return IOD_EOF;

-  return mio->pointer[offset];
+  memcpy (buf, &mio->pointer[offset], count);
+  return 0;
 }

 static int
-ios_dev_mem_putc (void *iod, int c, ios_dev_off offset)
+ios_dev_mem_pwrite (void *iod, const void *buf, size_t count,
+                   ios_dev_off offset)
+
 {
   struct ios_dev_mem *mio = iod;

-  if (offset >= mio->size) {
-    assert (MEM_STEP > offset - mio->size);
+  assert (count < MEM_STEP);
+  if (offset + count >= mio->size)
     mio->pointer = xrealloc (mio->pointer,
                              mio->size + MEM_STEP);
-  }
-  mio->pointer[offset] = c;
-  return c;
+  memcpy (&mio->pointer[offset], buf, count);
+  return 0;
 }

 static ios_dev_off
@@ -114,8 +117,8 @@ struct ios_dev_if ios_dev_mem =
    .handler_normalize = ios_dev_mem_handler_normalize,
    .open = ios_dev_mem_open,
    .close = ios_dev_mem_close,
-   .get_c = ios_dev_mem_getc,
-   .put_c = ios_dev_mem_putc,
+   .pread = ios_dev_mem_pread,
+   .pwrite = ios_dev_mem_pwrite,
    .get_flags = ios_dev_mem_get_flags,
    .size = ios_dev_mem_size,
   };
diff --git a/src/ios.c b/src/ios.c
index ca7b5b68..9873dd48 100644
--- a/src/ios.c
+++ b/src/ios.c
@@ -32,17 +32,19 @@

 #define STREQ(a, b) (strcmp (a, b) == 0)

-#define IOS_GET_C_ERR_CHCK(c, io, off)                 \
-  {                                                    \
-    int ret = io->dev_if->get_c ((io)->dev, off);      \
-    if (ret == IOD_EOF)                                        \
-      return IOS_EIOFF;                                        \
-    (c) = ret;                                         \
+#define IOS_GET_C_ERR_CHCK(c, io, off)                         \
+  {                                                            \
+    uint8_t ch;                                                        \
+    int ret = io->dev_if->pread ((io)->dev, &ch, 1, off);      \
+    if (ret == IOD_EOF)                                                \
+      return IOS_EIOFF;                                                \
+    (c) = ch;                                                  \
   }

 #define IOS_PUT_C_ERR_CHCK(c, io, off)                 \
   {                                                    \
-    if (io->dev_if->put_c ((io)->dev, (char)(c), off)  \
+    uint8_t ch = (c);                                  \
+    if (io->dev_if->pwrite ((io)->dev, &ch, 1, off)    \
        == IOD_EOF)                                     \
       return IOS_EIOBJ;                                        \
   }
@@ -910,7 +912,6 @@ ios_read_string (ios io, ios_off offset, int flags, char 
**value)
 {
   char *str = NULL;
   size_t i = 0;
-  int c;

   /* Apply the IOS bias.  */
   offset += ios_get_bias (io);
@@ -926,11 +927,9 @@ ios_read_string (ios io, ios_off offset, int flags, char 
**value)
           if (i % 128 == 0)
             str = xrealloc (str, i + 128 * sizeof (char));

-          c = io->dev_if->get_c (io->dev, offset / 8 + i);
-          if (c == IOD_EOF)
+          if (io->dev_if->pread (io->dev, &str[i], 1,
+                                offset / 8 + i) == IOD_EOF)
             return IOS_EIOFF;
-          else
-            str[i] = (char) c;
         }
       while (str[i++] != '\0');
     }
@@ -1511,8 +1510,8 @@ ios_write_string (ios io, ios_off offset, int flags,
       p = value;
       do
         {
-          if (io->dev_if->put_c (io->dev, *p,
-                                offset / 8 + p - value) == IOD_EOF)
+          if (io->dev_if->pwrite (io->dev, p, 1,
+                                 offset / 8 + p - value) == IOD_EOF)
             return IOS_EIOFF;
         }
       while (*(p++) != '\0');
-- 
2.25.1