Merge pull request #2109 in HDFFV/hdf5 from ~DEROBINS/hdf5_der:pread_bugs to develop

* commit '99f85cbf15a637768c8df977ddc5eebe7683a60d':
  Fixed bugs in pread/pwrite I/O in VFDs.

4 files changed, 36 insertions, 15 deletions

diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt
index 88be3de..9362da7 100644
--- a/release_docs/RELEASE.txt
+++ b/release_docs/RELEASE.txt
@@ -661,6 +661,21 @@ Bug Fixes since HDF5-1.10.3 release
 
       (DER - 2019/08/27, HDFFV-10892)
 
+    - Torn pread/pwrite I/O would result in read and write corruption.
+
+      In the sec2, log, and core (with backing store) virtual file drivers
+      (VFDs), the read and write calls incorrectly reset the offset parameter
+      on torn pread and pwrite operations (i.e., I/O operations which fail to
+      be written atomically by the OS). For this bug to occur, pread/pwrite
+      have to be configured (this is the default if they are present on the
+      system) and the pread/pwrite operation has to fail to transfer all
+      the bytes, resulting in a multiple pread/pwrite calls.
+
+      This feature was initially enabled in HDF5 1.10.5 so the bug is
+      limited to that version.
+
+      (DER - 2019/12/09, HDFFV-10945)
+
 
     Java Library:
     ----------------
diff --git a/src/H5FDcore.c b/src/H5FDcore.c
index 6db8af6..552b7ca 100644
--- a/src/H5FDcore.c
+++ b/src/H5FDcore.c
@@ -342,6 +342,7 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size)
     unsigned char  *ptr         = file->mem + addr;     /* mutable pointer into the
                                                          * buffer (can't change mem)
                                                          */
+    HDoff_t         offset      = (HDoff_t)addr;        /* Offset to write at */
     herr_t          ret_value   = SUCCEED;              /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -358,7 +359,6 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size)
 
         h5_posix_io_t       bytes_in        = 0;    /* # of bytes to write  */
         h5_posix_io_ret_t   bytes_wrote     = -1;   /* # of bytes written   */
-        HDoff_t             offset          = (HDoff_t)addr;
 
         /* Trying to write more bytes than the return type can handle is
          * undefined behavior in POSIX.
@@ -371,7 +371,8 @@ H5FD__core_write_to_bstore(H5FD_core_t *file, haddr_t addr, size_t size)
         do {
 #ifdef H5_HAVE_PREADWRITE
             bytes_wrote = HDpwrite(file->fd, ptr, bytes_in, offset);
-            offset += bytes_wrote;
+            if(bytes_wrote > 0)
+                offset += bytes_wrote;
 #else
             bytes_wrote = HDwrite(file->fd, ptr, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */
@@ -856,12 +857,12 @@ H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr
                  * partial results, and the end of the file.
                  */
                 
-                uint8_t *mem = file->mem; /* memory pointer for writes */
+                uint8_t *mem = file->mem;       /* memory pointer for writes */
+                HDoff_t offset = (HDoff_t)0;    /* offset for reading */
                 
                 while(size > 0) {
                     h5_posix_io_t       bytes_in        = 0;    /* # of bytes to read       */
                     h5_posix_io_ret_t   bytes_read      = -1;   /* # of bytes actually read */
-                    HDoff_t             offset          = (HDoff_t)0;
                     
                     /* Trying to read more bytes than the return type can handle is
                      * undefined behavior in POSIX.
@@ -874,7 +875,8 @@ H5FD__core_open(const char *name, unsigned flags, hid_t fapl_id, haddr_t maxaddr
                     do {
 #ifdef H5_HAVE_PREADWRITE
                         bytes_read = HDpread(file->fd, mem, bytes_in, offset);
-                        offset += bytes_read;
+                        if(bytes_read > 0)
+                            offset += bytes_read;
 #else
                         bytes_read = HDread(file->fd, mem, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */
diff --git a/src/H5FDlog.c b/src/H5FDlog.c
index ac5667f..60255e5 100644
--- a/src/H5FDlog.c
+++ b/src/H5FDlog.c
@@ -1171,6 +1171,7 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd
 #ifdef H5_HAVE_GETTIMEOFDAY
     struct timeval      timeval_start, timeval_stop;
 #endif /* H5_HAVE_GETTIMEOFDAY */
+    HDoff_t             offset = (HDoff_t)addr;
     herr_t              ret_value = SUCCEED;       /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -1255,7 +1256,6 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd
 
         h5_posix_io_t       bytes_in        = 0;    /* # of bytes to read       */
         h5_posix_io_ret_t   bytes_read      = -1;   /* # of bytes actually read */ 
-        HDoff_t             offset          = (HDoff_t)addr;
 
         /* Trying to read more bytes than the return type can handle is
          * undefined behavior in POSIX.
@@ -1268,7 +1268,8 @@ H5FD_log_read(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, hadd
         do {
 #ifdef H5_HAVE_PREADWRITE
             bytes_read = HDpread(file->fd, buf, bytes_in, offset);
-            offset += bytes_read;
+            if(bytes_read > 0)
+                offset += bytes_read;
 #else
             bytes_read = HDread(file->fd, buf, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */
@@ -1382,6 +1383,7 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had
 #ifdef H5_HAVE_GETTIMEOFDAY
     struct timeval      timeval_start, timeval_stop;
 #endif /* H5_HAVE_GETTIMEOFDAY */
+    HDoff_t             offset = (HDoff_t)addr;
     herr_t              ret_value = SUCCEED;       /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -1471,7 +1473,6 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had
 
         h5_posix_io_t       bytes_in        = 0;    /* # of bytes to write  */
         h5_posix_io_ret_t   bytes_wrote     = -1;   /* # of bytes written   */ 
-        HDoff_t             offset          = (HDoff_t)addr;
 
         /* Trying to write more bytes than the return type can handle is
          * undefined behavior in POSIX.
@@ -1484,7 +1485,8 @@ H5FD_log_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, had
         do {
 #ifdef H5_HAVE_PREADWRITE
             bytes_wrote = HDpwrite(file->fd, buf, bytes_in, offset);
-            offset += bytes_wrote;
+            if(bytes_wrote > 0)
+                offset += bytes_wrote;
 #else
             bytes_wrote = HDwrite(file->fd, buf, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */
diff --git a/src/H5FDsec2.c b/src/H5FDsec2.c
index 0054a86..a393490 100644
--- a/src/H5FDsec2.c
+++ b/src/H5FDsec2.c
@@ -669,6 +669,7 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS
     haddr_t addr, size_t size, void *buf /*out*/)
 {
     H5FD_sec2_t     *file       = (H5FD_sec2_t *)_file;
+    HDoff_t         offset      = (HDoff_t)addr;
     herr_t          ret_value   = SUCCEED;                  /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -687,7 +688,7 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS
     if(addr != file->pos || OP_READ != file->op) {
         if(HDlseek(file->fd, (HDoff_t)addr, SEEK_SET) < 0)
             HSYS_GOTO_ERROR(H5E_IO, H5E_SEEKERROR, FAIL, "unable to seek to proper position")
-    } /* end if */
+    }
 #endif /* H5_HAVE_PREADWRITE */
 
     /* Read data, being careful of interrupted system calls, partial results,
@@ -697,7 +698,6 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS
 
         h5_posix_io_t       bytes_in        = 0;    /* # of bytes to read       */
         h5_posix_io_ret_t   bytes_read      = -1;   /* # of bytes actually read */
-        HDoff_t             offset          = (HDoff_t)addr;
 
         /* Trying to read more bytes than the return type can handle is
          * undefined behavior in POSIX.
@@ -710,7 +710,8 @@ H5FD_sec2_read(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNUS
         do {
 #ifdef H5_HAVE_PREADWRITE
             bytes_read = HDpread(file->fd, buf, bytes_in, offset);
-            offset += bytes_read;
+            if(bytes_read > 0)
+                offset += bytes_read;
 #else
             bytes_read = HDread(file->fd, buf, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */
@@ -773,6 +774,7 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
                 haddr_t addr, size_t size, const void *buf)
 {
     H5FD_sec2_t     *file       = (H5FD_sec2_t *)_file;
+    HDoff_t         offset      = (HDoff_t)addr;
     herr_t          ret_value   = SUCCEED;                  /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT
@@ -791,7 +793,7 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
     if(addr != file->pos || OP_WRITE != file->op) {
         if(HDlseek(file->fd, (HDoff_t)addr, SEEK_SET) < 0)
             HSYS_GOTO_ERROR(H5E_IO, H5E_SEEKERROR, FAIL, "unable to seek to proper position")
-    } /* end if */
+    }
 #endif /* H5_HAVE_PREADWRITE */
 
     /* Write the data, being careful of interrupted system calls and partial
@@ -801,7 +803,6 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
 
         h5_posix_io_t       bytes_in        = 0;    /* # of bytes to write  */
         h5_posix_io_ret_t   bytes_wrote     = -1;   /* # of bytes written   */ 
-        HDoff_t             offset          = (HDoff_t)addr;
 
         /* Trying to write more bytes than the return type can handle is
          * undefined behavior in POSIX.
@@ -814,7 +815,8 @@ H5FD_sec2_write(H5FD_t *_file, H5FD_mem_t H5_ATTR_UNUSED type, hid_t H5_ATTR_UNU
         do {
 #ifdef H5_HAVE_PREADWRITE
             bytes_wrote = HDpwrite(file->fd, buf, bytes_in, offset);
-            offset += bytes_wrote;
+            if(bytes_wrote > 0)
+                offset += bytes_wrote;
 #else
             bytes_wrote = HDwrite(file->fd, buf, bytes_in);
 #endif /* H5_HAVE_PREADWRITE */