Fixes based on PR review comments:

For H5Dchunk.c: fix H5MM_xfree() For H5FDmpio.c: 1) Revert the fix to H5FD__mpio_write_vector() 2) Apply the patch from Neil on the proper length of s_sizes reported by H5FD__mpio_vector_build_types()
HDFGroup · Nov 3, 2023 · 5b501b9 · 5b501b9
1 parent f018960
commit 5b501b9
Show file tree

Hide file tree

Showing 2 changed files with 19 additions and 14 deletions.
diff --git a/src/H5Dchunk.c b/src/H5Dchunk.c
@@ -5621,8 +5621,7 @@ H5D__chunk_collective_fill(const H5D_t *dset, H5D_chunk_coll_fill_info_t *chunk_
 
     H5MM_xfree(io_addrs);
     H5MM_xfree(io_wbufs);
-    if (!all_same_block_len)
-        H5MM_xfree(io_sizes);
+    H5MM_xfree(io_sizes);
 
     FUNC_LEAVE_NOAPI(ret_value)
 } /* end H5D__chunk_collective_fill() */

diff --git a/src/H5FDmpio.c b/src/H5FDmpio.c
@@ -106,7 +106,7 @@ static herr_t H5FD__mpio_ctl(H5FD_t *_file, uint64_t op_code, uint64_t flags, co
 /* Other functions */
 static herr_t H5FD__mpio_vector_build_types(uint32_t count, H5FD_mem_t types[], haddr_t addrs[],
                                             size_t sizes[], H5_flexible_const_ptr_t bufs[],
-                                            haddr_t *s_addrs[], size_t *s_sizes[],
+                                            haddr_t *s_addrs[], size_t *s_sizes[], uint32_t *s_sizes_len,
                                             H5_flexible_const_ptr_t *s_bufs[], bool *vector_was_sorted,
                                             MPI_Offset *mpi_off, H5_flexible_const_ptr_t *mpi_bufs_base,
                                             int *size_i, MPI_Datatype *buf_type, bool *buf_type_created,
@@ -1674,7 +1674,7 @@ H5FD__mpio_write(H5FD_t *_file, H5FD_mem_t type, hid_t H5_ATTR_UNUSED dxpl_id, h
  */
 static herr_t
 H5FD__mpio_vector_build_types(uint32_t count, H5FD_mem_t types[], haddr_t addrs[], size_t sizes[],
-                              H5_flexible_const_ptr_t bufs[], haddr_t *s_addrs[], size_t *s_sizes[],
+                              H5_flexible_const_ptr_t bufs[], haddr_t *s_addrs[], size_t *s_sizes[], uint32_t *s_sizes_len,
                               H5_flexible_const_ptr_t *s_bufs[], bool *vector_was_sorted, MPI_Offset *mpi_off,
                               H5_flexible_const_ptr_t *mpi_bufs_base, int *size_i, MPI_Datatype *buf_type,
                               bool *buf_type_created, MPI_Datatype *file_type, bool *file_type_created,
@@ -1716,6 +1716,10 @@ H5FD__mpio_vector_build_types(uint32_t count, H5FD_mem_t types[], haddr_t addrs[
     /* Get bio I/O transition point (may be lower than 2G for testing) */
     bigio_count = H5_mpi_get_bigio_count();
 
+    /* Start wiht s_sizes_len at count */
+    if (s_sizes_len)
+        *s_sizes_len = count;
+
     if (count == 1) {
         /* Single block.  Just use a series of MPI_BYTEs for the file view.
          */
@@ -1808,8 +1812,13 @@ H5FD__mpio_vector_build_types(uint32_t count, H5FD_mem_t types[], haddr_t addrs[
             if (!fixed_size) {
                 if ((*s_sizes)[i] == 0) {
                     assert(vector_was_sorted);
+                    assert(i > 0);
                     fixed_size = true;
                     size       = sizes[i - 1];
+
+                    /* Return the used length of the s_sizes buffer */
+                    if (s_sizes_len)
+                        *s_sizes_len = (uint32_t)i;
                 }
                 else {
                     size = (*s_sizes)[i];
@@ -2098,7 +2107,7 @@ H5FD__mpio_read_vector(H5FD_t *_file, hid_t H5_ATTR_UNUSED dxpl_id, uint32_t cou
     if (xfer_mode == H5FD_MPIO_COLLECTIVE) {
         /* Build MPI types, etc. */
         if (H5FD__mpio_vector_build_types(count, types, addrs, sizes, (H5_flexible_const_ptr_t *)bufs,
-                                          &s_addrs, &s_sizes, (H5_flexible_const_ptr_t **)&s_bufs,
+                                          &s_addrs, &s_sizes, NULL, (H5_flexible_const_ptr_t **)&s_bufs,
                                           &vector_was_sorted, &mpi_off,
                                           (H5_flexible_const_ptr_t *)&mpi_bufs_base, &size_i, &buf_type,
                                           &buf_type_created, &file_type, &file_type_created, &unused) < 0)
@@ -2464,22 +2473,19 @@ H5FD__mpio_write_vector(H5FD_t *_file, hid_t H5_ATTR_UNUSED dxpl_id, uint32_t co
         HGOTO_ERROR(H5E_VFL, H5E_CANTGET, FAIL, "can't get MPI-I/O transfer mode");
 
     if (xfer_mode == H5FD_MPIO_COLLECTIVE) {
+        uint32_t s_sizes_len;
+
         /* Build MPI types, etc. */
         if (H5FD__mpio_vector_build_types(count, types, addrs, sizes, (H5_flexible_const_ptr_t *)bufs,
-                                          &s_addrs, &s_sizes, (H5_flexible_const_ptr_t **)&s_bufs,
+                                          &s_addrs, &s_sizes, &s_sizes_len, (H5_flexible_const_ptr_t **)&s_bufs,
                                           &vector_was_sorted, &mpi_off,
                                           (H5_flexible_const_ptr_t *)&mpi_bufs_base, &size_i, &buf_type,
                                           &buf_type_created, &file_type, &file_type_created, &unused) < 0)
             HGOTO_ERROR(H5E_VFL, H5E_CANTGET, FAIL, "can't build MPI datatypes for I/O");
 
-        /* Compute max address written to */
-        /* Bug: need to account for fixed size optimization */
-        if (count > 0) {
-            uint32_t idx = count - 1;
-            if (s_sizes[0] != 0 && s_sizes[1] == 0)
-                idx = 0;
-            max_addr = s_addrs[count - 1] + (haddr_t)(s_sizes[idx]);
-        }
+        /* Compute max address written to.  Note s_sizes is indexed according to the length of that array as reported by H5FD__mpio_vector_build_types(), which may be shorter if using the compressed arrays feature. */
+        if (count > 0)
+            max_addr = s_addrs[count - 1] + (haddr_t)(s_sizes[s_sizes_len - 1]);
 
         /* free sorted vectors if they exist */
         if (!vector_was_sorted) {