Various improvement in documentation and a decoding function (#6383)

* Various improvement in documentation and a decoding function

- Improves documentation on the type size when creating/accessing a compound datatype with no predefined struct (GH issue #5371)
- Provides better description of the min_meta_perc and min_raw_perc arguments in the H5Pset_page_buffer_size() (GH issue #5711)
- Adds error checkings to an internal decoding function

* Corrected the checks, the base address equals the end-of-file is valid.

* Skip for multi-file and split drivers when validating addresses against stored_eof

* Committing clang-format changes

* Remove incorrect name

* Fix typos

* Modified description of type_size arguments

* Used a more robust condition when checking EOF

* Corrected incorrect conflict resolving

* Update src/H5Fsuper_cache.c

Co-authored-by: Neil Fortner <fortnern@gmail.com>

* Update src/H5Fsuper_cache.c

Co-authored-by: Neil Fortner <fortnern@gmail.com>

* Committing clang-format changes

* Modified per feedbacks.

* Fix comparison per feedbacks

* Omit unused parameter name in a catch block

---------

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Neil Fortner <fortnern@gmail.com>

c++/src/H5StrType.cpp

@@ -194,7 +194,7 @@ StrType::decode() const
     try {
         encoded_strtype_id = p_decode();
     }
-    catch (DataTypeIException &err) {
+    catch (DataTypeIException &) {
         throw;
     }
     StrType *encoded_strtype = new StrType;

c++/src/H5VarLenType.cpp

@@ -138,7 +138,7 @@ VarLenType::decode() const
     try {
         encoded_vltype_id = p_decode();
     }
-    catch (DataTypeIException &err) {
+    catch (DataTypeIException &) {
         throw;
     }
     VarLenType *encoded_vltype = new VarLenType;

hl/src/H5TBpublic.h

@@ -139,8 +139,7 @@ extern "C" {
  * \param[in] nfields       The number of fields
  * \param[in] nrecords      The number of records
  * \param[in] type_size     The size in bytes of the structure
- *                          associated with the table;
+ *                          associated with the table
- *                          This value is obtained with \c sizeof().
  * \param[in] field_names   An array containing the names of the fields.
  *                          Names longer than #HLTB_MAX_FIELD_LEN - 1 characters
  *                          are silently truncated when read back by
@@ -160,6 +159,12 @@ extern "C" {
  *          \p dset_name attached to the object specified by the
  *          identifier loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct.  Otherwise, \p type_size should
+ *          be calculated based on the highest offset in \p field_offset, the
+ *          size of its corresponding datatype in \p field_types, and any
+ *          padding bytes desired after that field.
+ *
  */
 H5HL_DLL herr_t H5TBmake_table(const char *table_title, hid_t loc_id, const char *dset_name, hsize_t nfields,
                                hsize_t nrecords, size_t type_size, const char *field_names[],
@@ -182,8 +187,7 @@ H5HL_DLL herr_t H5TBmake_table(const char *table_title, hid_t loc_id, const char
  * \fg_loc_id
  * \param[in] dset_name     The name of the dataset to overwrite
  * \param[in] nrecords      The number of records to append
- * \param[in] type_size     The size of the structure type,
+ * \param[in] type_size     The size of the structure type
- *                          as calculated by \c sizeof().
  * \param[in] field_offset  An array containing the offsets of
  *                          the fields. These offsets can be
  *                          calculated with the #HOFFSET macro
@@ -198,6 +202,10 @@ H5HL_DLL herr_t H5TBmake_table(const char *table_title, hid_t loc_id, const char
  *          identifier \p loc_id. The dataset is extended to hold the
  *          new records.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBappend_records(hid_t loc_id, const char *dset_name, hsize_t nrecords, size_t type_size,
                                    const size_t *field_offset, const size_t *dst_sizes, const void *buf);
@@ -212,8 +220,7 @@ H5HL_DLL herr_t H5TBappend_records(hid_t loc_id, const char *dset_name, hsize_t
  * \param[in] dset_name    The name of the dataset to overwrite
  * \param[in] start         The zero index record to start writing
  * \param[in] nrecords      The number of records to write
- * \param[in] type_size     The size of the structure type, as
+ * \param[in] type_size     The size of the structure type
- *                          calculated by \c sizeof().
  * \param[in] field_offset  An array containing the offsets of
  *                          the fields.  These offsets can be
  *                          calculated with the #HOFFSET macro
@@ -227,6 +234,10 @@ H5HL_DLL herr_t H5TBappend_records(hid_t loc_id, const char *dset_name, hsize_t
  *          index position start of the table named \p dset_name attached
  *          to the object specified by the identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBwrite_records(hid_t loc_id, const char *dset_name, hsize_t start, hsize_t nrecords,
                                   size_t type_size, const size_t *field_offset, const size_t *dst_sizes,
@@ -243,8 +254,7 @@ H5HL_DLL herr_t H5TBwrite_records(hid_t loc_id, const char *dset_name, hsize_t s
  * \param[in] field_names   The names of the fields to write
  * \param[in] start         The zero index record to start writing
  * \param[in] nrecords      The number of records to write
- * \param[in] type_size     The size of the structure type, as
+ * \param[in] type_size     The size of the structure type
- *                          calculated by \c sizeof().
  * \param[in] field_offset  An array containing the offsets of
  *                          the fields.  These offsets can be
  *                          calculated with the #HOFFSET macro
@@ -259,6 +269,10 @@ H5HL_DLL herr_t H5TBwrite_records(hid_t loc_id, const char *dset_name, hsize_t s
  *          dataset named \p dset_name attached to the object specified
  *          by the identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBwrite_fields_name(hid_t loc_id, const char *dset_name, const char *field_names,
                                       hsize_t start, hsize_t nrecords, size_t type_size,
@@ -278,8 +292,7 @@ H5HL_DLL herr_t H5TBwrite_fields_name(hid_t loc_id, const char *dset_name, const
  * \param[in] field_index   The indexes of the fields to write
  * \param[in] start         The zero based index record to start writing
  * \param[in] nrecords      The number of records to write
- * \param[in] type_size     The size of the structure type, as
+ * \param[in] type_size     The size of the structure type
- *                          calculated by \c sizeof().
  * \param[in] field_offset  An array containing the offsets of
  *                          the fields.  These offsets can be
  *                          calculated with the #HOFFSET macro
@@ -294,6 +307,10 @@ H5HL_DLL herr_t H5TBwrite_fields_name(hid_t loc_id, const char *dset_name, const
  *          dataset named \p dset_name attached to the object
  *          specified by the identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBwrite_fields_index(hid_t loc_id, const char *dset_name, hsize_t nfields,
                                        const int *field_index, hsize_t start, hsize_t nrecords,
@@ -315,8 +332,7 @@ H5HL_DLL herr_t H5TBwrite_fields_index(hid_t loc_id, const char *dset_name, hsiz
  *
  * \fg_loc_id
  * \param[in] dset_name    The name of the dataset to read
- * \param[in] dst_size     The size of the structure type,
+ * \param[in] dst_size     The size of the structure type
- *                          as calculated by \c sizeof()
  * \param[in] dst_offset    An array containing the offsets of
  *                          the fields.  These offsets can be
  *                          calculated with the #HOFFSET macro
@@ -331,6 +347,10 @@ H5HL_DLL herr_t H5TBwrite_fields_index(hid_t loc_id, const char *dset_name, hsiz
  *          \p dset_name attached to the object specified by
  *          the identifier \p loc_id.
  *
+ *          \p dst_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBread_table(hid_t loc_id, const char *dset_name, size_t dst_size, const size_t *dst_offset,
                                const size_t *dst_sizes, void *dst_buf);
@@ -349,7 +369,6 @@ H5HL_DLL herr_t H5TBread_table(hid_t loc_id, const char *dset_name, size_t dst_s
  * \param[in] nrecords      The number of records to read
  * \param[in] type_size     The size in bytes of the structure associated
  *                          with the table
- *                          (This value is obtained with \c sizeof().)
  * \param[in] field_offset  An array containing the offsets of the fields
  * \param[in] dst_sizes     An array containing the size in bytes of
  *                          the fields
@@ -361,6 +380,10 @@ H5HL_DLL herr_t H5TBread_table(hid_t loc_id, const char *dset_name, size_t dst_s
  *          by \p field_names from a dataset named \p dset_name
  *          attached to the object specified by the identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBread_fields_name(hid_t loc_id, const char *dset_name, const char *field_names,
                                      hsize_t start, hsize_t nrecords, size_t type_size,
@@ -384,7 +407,6 @@ H5HL_DLL herr_t H5TBread_fields_name(hid_t loc_id, const char *dset_name, const
  * \param[in] nrecords      The number of records to read
  * \param[in] type_size     The size in bytes of the structure associated
  *                          with the table
- *                          (This value is obtained with \c sizeof())
  * \param[in] field_offset  An array containing the offsets of the fields
  * \param[in] dst_sizes     An array containing the size in bytes of
  *                          the fields
@@ -396,6 +418,10 @@ H5HL_DLL herr_t H5TBread_fields_name(hid_t loc_id, const char *dset_name, const
  *          by \p field_index from a dataset named \p dset_name attached
  *          to the object specified by the identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBread_fields_index(hid_t loc_id, const char *dset_name, hsize_t nfields,
                                       const int *field_index, hsize_t start, hsize_t nrecords,
@@ -413,8 +439,7 @@ H5HL_DLL herr_t H5TBread_fields_index(hid_t loc_id, const char *dset_name, hsize
  * \param[in] dset_name     The name of the dataset to read
  * \param[in] start         The start record to read from
  * \param[in] nrecords      The number of records to read
- * \param[in] type_size     The size of the structure type,
+ * \param[in] type_size     The size of the structure type
- *                          as calculated by \c sizeof()
  * \param[in] dst_offset    An array containing the offsets of the
  *                          fields.  These offsets can be calculated
  *                          with the #HOFFSET macro
@@ -428,6 +453,10 @@ H5HL_DLL herr_t H5TBread_fields_index(hid_t loc_id, const char *dset_name, hsize
  *          named \p dset_name attached to the object specified by the
  *          identifier \p loc_id.
  *
+ *          \p type_size can be obtained with \c sizeof(), if the data is
+ *          stored in a predefined C struct. Otherwise, it can be obtained
+ *          by calling H5TBget_field_info() if not already known.
+ *
  */
 H5HL_DLL herr_t H5TBread_records(hid_t loc_id, const char *dset_name, hsize_t start, hsize_t nrecords,
                                  size_t type_size, const size_t *dst_offset, const size_t *dst_sizes,
@@ -549,6 +578,10 @@ H5HL_DLL herr_t H5TBdelete_record(hid_t loc_id, const char *dset_name, hsize_t s
  * \details H5TBinsert_record() inserts records into the middle of the table
  *          ("pushing down" all the records after it)
  *
+ *          \p dst_size can be obtained with \c sizeof() if the predefined C
+ *          structure is available, otherwise, use H5Tget_size() on the
+ *          compound datatype.
+ *
  */
 H5HL_DLL herr_t H5TBinsert_record(hid_t loc_id, const char *dset_name, hsize_t start, hsize_t nrecords,
                                   size_t dst_size, const size_t *dst_offset, const size_t *dst_sizes,

src/H5FDonion.c

@@ -1154,7 +1154,7 @@ H5FD__onion_open(const char *filename, unsigned flags, hid_t fapl_id, haddr_t ma
          * We're getting this buffer from a fixed-size array in a struct, which
          * will be garbage and not null-terminated if the user isn't careful.
          * Be careful of this and do strndup first to ensure strdup gets a
-         * null-termianted string (HDF5 doesn't provide a strnlen call if you
+         * null-terminated string (HDF5 doesn't provide a strnlen call if you
          * don't have one).
          */
         if (NULL ==

src/H5Fsuper_cache.c

@@ -521,12 +521,36 @@ H5F__cache_superblock_deserialize(const void *_image, size_t len, void *_udata,
         udata->btree_k[H5B_CHUNK_ID] = chunk_btree_k;
 
         /* Remainder of "variable-sized" portion of superblock */
+
+        /* Check whether the image pointer will be out of bounds */
         if (H5_IS_BUFFER_OVERFLOW(image, H5F_sizeof_addr(udata->f) * 4, end))
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
+
+        /* Get and verify base address, delay additional verification */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
+        if (!H5_addr_defined(sblock->base_addr))
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "base address is undefined");
+
+        /* Get extension address, delay verification until stored eof is avail */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
+
+        /* Get and verify stored eof */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &udata->stored_eof /*out*/);
+        if (!H5_addr_defined(udata->stored_eof))
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "stored EOF address is undefined");
+        if (udata->stored_eof == 0)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "stored EOF address cannot be 0");
+
+        /* Get driver address */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->driver_addr /*out*/);
+        if (H5_addr_defined(sblock->driver_addr) && sblock->driver_addr >= udata->stored_eof)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "driver info block address exceeds end of file");
+
+        /* Validate base and extension addresses against stored_eof */
+        if (sblock->base_addr >= udata->stored_eof)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "base address exceeds stored EOF");
+        if (H5_addr_defined(sblock->ext_addr) && sblock->ext_addr >= udata->stored_eof)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "superblock extension address exceeds stored EOF");
 
         /* Allocate space for the root group symbol table entry */
         if (sblock->root_ent)
@@ -579,10 +603,40 @@ H5F__cache_superblock_deserialize(const void *_image, size_t len, void *_udata,
             HGOTO_ERROR(H5E_FILE, H5E_OVERFLOW, NULL, "image pointer is out of bounds");
 
         /* Base, superblock extension, end of file & root group object header addresses */
+
+        /* Get and verify base address, delay additional verification */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->base_addr /*out*/);
+        if (!H5_addr_defined(sblock->base_addr))
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "base address is undefined");
+
+        /* Get extension address, delay verification until stored eof is avail */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->ext_addr /*out*/);
+
+        /* Get and verify stored eof */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &udata->stored_eof /*out*/);
+        if (!H5_addr_defined(udata->stored_eof))
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "stored EOF address is undefined");
+        if (udata->stored_eof == 0)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "stored EOF address cannot be 0");
+
+        /* Get and verify root address */
         H5F_addr_decode(udata->f, (const uint8_t **)&image, &sblock->root_addr /*out*/);
+        if (!H5_addr_defined(sblock->root_addr))
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root address is undefined");
+        if (sblock->root_addr == 0)
+            HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root address cannot be 0");
+
+        /* Validate addresses against stored_eof.
+           Skip for VFDs that don't use absolute file offsets */
+        if (H5F_HAS_FEATURE(udata->f, H5FD_FEAT_DEFAULT_VFD_COMPATIBLE)) {
+            if (sblock->base_addr >= udata->stored_eof)
+                HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "base address exceeds stored EOF");
+            if (sblock->root_addr >= (udata->stored_eof - sblock->base_addr))
+                HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "root group address beyond stored EOF");
+            if (H5_addr_defined(sblock->ext_addr) &&
+                sblock->ext_addr >= (udata->stored_eof - sblock->base_addr))
+                HGOTO_ERROR(H5E_FILE, H5E_BADVALUE, NULL, "superblock extension address exceeds stored EOF");
+        }
 
         /* checksum verification already done in verify_chksum cb */
 

src/H5Ppublic.h

@@ -5763,6 +5763,17 @@ H5_DLL herr_t H5Pset_mdc_image_config(hid_t plist_id, H5AC_cache_image_config_t
  *          larger than the page buffer size, the subsequent call to H5Fcreate()
  *          using the \p plist_id will fail.
  *
+ *          The arguments min_meta_perc and min_raw_perc are to prevent one type
+ *          of data from evicting hot pages of the other type, that is, pushing
+ *          them out of the buffer.  Setting a minimum percentage for each type
+ *          reserves a portion of the page buffer for that type, ensuring both
+ *          metadata and raw data can maintain a presence in the buffer.
+ *
+ *          The following constraints apply to min_meta_perc and min_raw_perc:
+ *          - Each must be between 0 and 100 inclusive
+ *          - Their sum must not exceed 100, that is, together they can't reserve
+ *            more than the entire page buffer.
+ *
  * \note    As of HDF5 1.14.4, this property will be ignored when an existing
  *          file is being opened and the file space strategy stored in the
  *          file isn't paged. This was previously a failure.

src/H5VM.c

@@ -1308,7 +1308,7 @@ done:
  * Function:	H5VM_memcpyvv
  *
  * Purpose:     Given source and destination buffers in memory (SRC & DST)
- *              copy sequences of from the source buffer into the destination
+ *              copy sequences from the source buffer into the destination
  *              buffer.  Each set of sequences has an array of lengths, an
  *              array of offsets, the maximum number of sequences and the
  *              current sequence to start at in the sequence.