Refactor datatype conversion code to use pointers rather than IDs (#4104)

The datatype conversion code previously used IDs for the source and
destination datatypes rather than pointers to the internal structures
for those datatypes. This was mostly due to the need for an ID for these
datatypes that can be passed to an application-registered datatype
conversion function or datatype conversion exception function. However,
using IDs internally caused a lot of unnecessary ID lookups and hurt
performance of datatype conversions in general. This was especially
problematic for compound datatype conversions, where the ID lookups were
occuring on every member of every compound element of a dataset. The
code has now been refactored to use pointers internally and only create
IDs for datatypes when necessary.

Fixed a test issue in dt_arith where a library datatype conversion
function was being cast to an application conversion function. Since the
two have different prototypes, this started failing after the parameters
for a library conversion function changed from hid_t to H5T_t * and an
extra parameter was added. This appears to have worked coincidentally in
the past since the only different between a library conversion function
and application conversion function was an extra DXPL parameter at the
end of an application conversion function

Fixed an issue where memory wasn't being freed in the h5fc_chk_idx test
program. Even though the program exits quickly after allocating the
memory, it still causes failures when testing with -fsanitize=address

1 files changed, 23 insertions, 32 deletions

diff --git a/src/H5Dint.c b/src/H5Dint.c
index b37d35c..a267468 100644
--- a/src/H5Dint.c
+++ b/src/H5Dint.c
@@ -88,7 +88,7 @@ static herr_t H5D__use_minimized_dset_headers(H5F_t *file, bool *minimize);
 static herr_t H5D__prepare_minimized_oh(H5F_t *file, H5D_t *dset, H5O_loc_t *oloc);
 static size_t H5D__calculate_minimum_header_size(H5F_t *file, H5D_t *dset, H5O_t *ohdr);
 static void  *H5D__vlen_get_buf_size_alloc(size_t size, void *info);
-static herr_t H5D__vlen_get_buf_size_cb(void *elem, hid_t type_id, unsigned ndim, const hsize_t *point,
+static herr_t H5D__vlen_get_buf_size_cb(void *elem, H5T_t *type, unsigned ndim, const hsize_t *point,
                                         void *op_data);
 static herr_t H5D__vlen_get_buf_size_gen_cb(void *elem, hid_t type_id, unsigned ndim, const hsize_t *point,
                                             void *op_data);
@@ -2624,7 +2624,7 @@ done:
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5D__vlen_get_buf_size_cb(void H5_ATTR_UNUSED *elem, hid_t type_id, unsigned H5_ATTR_UNUSED ndim,
+H5D__vlen_get_buf_size_cb(void H5_ATTR_UNUSED *elem, H5T_t *type, unsigned H5_ATTR_UNUSED ndim,
                           const hsize_t *point, void *op_data)
 {
     H5D_vlen_bufsize_native_t *vlen_bufsize = (H5D_vlen_bufsize_native_t *)op_data;
@@ -2634,7 +2634,7 @@ H5D__vlen_get_buf_size_cb(void H5_ATTR_UNUSED *elem, hid_t type_id, unsigned H5_
     FUNC_ENTER_PACKAGE
 
     /* Sanity check */
-    assert(H5I_DATATYPE == H5I_get_type(type_id));
+    assert(type);
     assert(point);
     assert(op_data);
 
@@ -2643,11 +2643,11 @@ H5D__vlen_get_buf_size_cb(void H5_ATTR_UNUSED *elem, hid_t type_id, unsigned H5_
         HGOTO_ERROR(H5E_DATASET, H5E_CANTCREATE, H5_ITER_ERROR, "can't select point");
 
     {
-        dset_info.dset        = vlen_bufsize->dset;
-        dset_info.mem_space   = vlen_bufsize->mspace;
-        dset_info.file_space  = vlen_bufsize->fspace;
-        dset_info.buf.vp      = vlen_bufsize->common.fl_tbuf;
-        dset_info.mem_type_id = type_id;
+        dset_info.dset       = vlen_bufsize->dset;
+        dset_info.mem_space  = vlen_bufsize->mspace;
+        dset_info.file_space = vlen_bufsize->fspace;
+        dset_info.buf.vp     = vlen_bufsize->common.fl_tbuf;
+        dset_info.mem_type   = type;
 
         /* Read in the point (with the custom VL memory allocator) */
         if (H5D__read(1, &dset_info) < 0)
@@ -2729,9 +2729,8 @@ H5D__vlen_get_buf_size(H5D_t *dset, hid_t type_id, hid_t space_id, hsize_t *size
     vlen_bufsize.common.size = 0;
 
     /* Call H5S_select_iterate with args, etc. */
-    dset_op.op_type          = H5S_SEL_ITER_OP_APP;
-    dset_op.u.app_op.op      = H5D__vlen_get_buf_size_cb;
-    dset_op.u.app_op.type_id = type_id;
+    dset_op.op_type  = H5S_SEL_ITER_OP_LIB;
+    dset_op.u.lib_op = H5D__vlen_get_buf_size_cb;
 
     ret_value = H5S_select_iterate(&bogus, type, space, &dset_op, &vlen_bufsize);
 
@@ -3561,6 +3560,8 @@ H5D_get_create_plist(const H5D_t *dset)
     H5O_layout_t    copied_layout;     /* Layout to tweak */
     H5O_fill_t      copied_fill = {0}; /* Fill value to tweak */
     H5O_efl_t       copied_efl;        /* External file list to tweak */
+    H5T_t          *src_type    = NULL;
+    H5T_t          *dst_type    = NULL;
     hid_t           new_dcpl_id = FAIL;
     hid_t           ret_value   = H5I_INVALID_HID; /* Return value */
 
@@ -3646,43 +3647,28 @@ H5D_get_create_plist(const H5D_t *dset)
 
         /* Convert disk form of fill value into memory form */
         if (!H5T_path_noop(tpath)) {
-            hid_t    dst_id, src_id; /* Source & destination datatypes for type conversion */
             uint8_t *bkg_buf = NULL; /* Background conversion buffer */
             size_t   bkg_size;       /* Size of background buffer */
 
-            /* Wrap copies of types to convert */
-            dst_id = H5I_register(H5I_DATATYPE, H5T_copy(copied_fill.type, H5T_COPY_TRANSIENT), false);
-            if (dst_id < 0)
-                HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINIT, FAIL, "unable to copy/register datatype");
-            src_id = H5I_register(H5I_DATATYPE, H5T_copy(dset->shared->type, H5T_COPY_ALL), false);
-            if (src_id < 0) {
-                H5I_dec_ref(dst_id);
-                HGOTO_ERROR(H5E_DATASET, H5E_CANTINIT, FAIL, "unable to copy/register datatype");
-            } /* end if */
+            if (NULL == (src_type = H5T_copy(dset->shared->type, H5T_COPY_ALL)))
+                HGOTO_ERROR(H5E_DATASET, H5E_CANTCOPY, FAIL, "unable to copy dataset's datatype");
+            if (NULL == (dst_type = H5T_copy(copied_fill.type, H5T_COPY_TRANSIENT)))
+                HGOTO_ERROR(H5E_DATASET, H5E_CANTCOPY, FAIL, "unable to copy fill value datatype");
 
             /* Allocate a background buffer */
             bkg_size = MAX(H5T_GET_SIZE(copied_fill.type), H5T_GET_SIZE(dset->shared->type));
-            if (H5T_path_bkg(tpath) && NULL == (bkg_buf = H5FL_BLK_CALLOC(type_conv, bkg_size))) {
-                H5I_dec_ref(src_id);
-                H5I_dec_ref(dst_id);
+            if (H5T_path_bkg(tpath) && NULL == (bkg_buf = H5FL_BLK_CALLOC(type_conv, bkg_size)))
                 HGOTO_ERROR(H5E_DATASET, H5E_CANTALLOC, FAIL, "memory allocation failed");
-            } /* end if */
 
             /* Convert fill value */
-            if (H5T_convert(tpath, src_id, dst_id, (size_t)1, (size_t)0, (size_t)0, copied_fill.buf,
+            if (H5T_convert(tpath, src_type, dst_type, (size_t)1, (size_t)0, (size_t)0, copied_fill.buf,
                             bkg_buf) < 0) {
-                H5I_dec_ref(src_id);
-                H5I_dec_ref(dst_id);
                 if (bkg_buf)
                     bkg_buf = H5FL_BLK_FREE(type_conv, bkg_buf);
                 HGOTO_ERROR(H5E_DATASET, H5E_CANTCONVERT, FAIL, "datatype conversion failed");
             } /* end if */
 
             /* Release local resources */
-            if (H5I_dec_ref(src_id) < 0)
-                HGOTO_ERROR(H5E_DATASET, H5E_CANTDEC, FAIL, "unable to close temporary object");
-            if (H5I_dec_ref(dst_id) < 0)
-                HGOTO_ERROR(H5E_DATASET, H5E_CANTDEC, FAIL, "unable to close temporary object");
             if (bkg_buf)
                 bkg_buf = H5FL_BLK_FREE(type_conv, bkg_buf);
         } /* end if */
@@ -3713,6 +3699,11 @@ H5D_get_create_plist(const H5D_t *dset)
     ret_value = new_dcpl_id;
 
 done:
+    if (src_type && (H5T_close(src_type) < 0))
+        HDONE_ERROR(H5E_DATASET, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
+    if (dst_type && (H5T_close(dst_type) < 0))
+        HDONE_ERROR(H5E_DATASET, H5E_CANTCLOSEOBJ, FAIL, "unable to close temporary datatype");
+
     if (ret_value < 0) {
         if (new_dcpl_id > 0)
             if (H5I_dec_app_ref(new_dcpl_id) < 0)