From 921b53612d9bb4eacfddfdd2dbd6bee509b4a79b Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Thu, 6 Jun 2024 11:56:48 -0500 Subject: [PATCH 1/2] Simplify property copying internally --- src/H5Pint.c | 51 ++++++++++++++++++++++----------------------------- 1 file changed, 22 insertions(+), 29 deletions(-) diff --git a/src/H5Pint.c b/src/H5Pint.c index 8f9f5125847..032bfdce038 100644 --- a/src/H5Pint.c +++ b/src/H5Pint.c @@ -4862,15 +4862,16 @@ H5P_remove(H5P_genplist_t *plist, const char *name) DESCRIPTION Copies a property from one property list to another. - If a property is copied from one list to another, the property will be - first deleted from the destination list (generating a call to the 'close' + If the property exists in the destination list, the property will be + first deleted from the destination list (generating a call to the 'del' callback for the property, if one exists) and then the property is copied from the source list to the destination list (generating a call to the 'copy' callback for the property, if one exists). - If the property does not exist in the destination list, this call is - equivalent to calling H5Pinsert2 and the 'create' callback will be called - (if such a callback exists for the property). + If the property does not exist in the destination list, a new instance + of the property is created, the 'create' callback is called + (if such a callback exists for the property), and the new property is + inserted into the destination list. GLOBAL VARIABLES COMMENTS, BUGS, ASSUMPTIONS @@ -4895,15 +4896,16 @@ H5P__copy_prop_plist(hid_t dst_id, hid_t src_id, const char *name) NULL == (dst_plist = (H5P_genplist_t *)H5I_object(dst_id))) HGOTO_ERROR(H5E_PLIST, H5E_NOTFOUND, FAIL, "property object doesn't exist"); + /* Get the pointer to the source property */ + if (NULL == (prop = H5P__find_prop_plist(src_plist, name))) + HGOTO_ERROR(H5E_PLIST, H5E_NOTFOUND, FAIL, "property doesn't exist"); + /* If the property exists in the destination already */ if (NULL != H5P__find_prop_plist(dst_plist, name)) { - /* Delete the property from the destination list, calling the 'close' callback if necessary */ + /* Delete the property from the destination list, calling the 'del' callback if necessary */ if (H5P_remove(dst_plist, name) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTDELETE, FAIL, "unable to remove property"); - /* Get the pointer to the source property */ - prop = H5P__find_prop_plist(src_plist, name); - /* Make a copy of the source property */ if ((new_prop = H5P__dup_prop(prop, H5P_PROP_WITHIN_LIST)) == NULL) HGOTO_ERROR(H5E_PLIST, H5E_CANTCOPY, FAIL, "Can't copy property"); @@ -4913,21 +4915,12 @@ H5P__copy_prop_plist(hid_t dst_id, hid_t src_id, const char *name) if ((new_prop->copy)(new_prop->name, new_prop->size, new_prop->value) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTCOPY, FAIL, "Can't copy property"); } /* end if */ - - /* Insert the initialized property into the property list */ - if (H5P__add_prop(dst_plist->props, new_prop) < 0) - HGOTO_ERROR(H5E_PLIST, H5E_CANTINSERT, FAIL, "Can't insert property into list"); - - /* Increment the number of properties in list */ - dst_plist->nprops++; - } /* end if */ - /* If not, get the information required to do an H5Pinsert2 with the property into the destination list */ + } /* end if */ + /* If property doesn't exist in destination */ else { - /* Get the pointer to the source property */ - if (NULL == (prop = H5P__find_prop_plist(src_plist, name))) - HGOTO_ERROR(H5E_PLIST, H5E_NOTFOUND, FAIL, "property doesn't exist"); - - /* Create property object from parameters */ + /* Create property object from parameters. This is very similar to the property + * duplication call above, but the property's name is treated differently depending on + * whether the source property is defined on a plist or a plist class */ if (NULL == (new_prop = H5P__create_prop(prop->name, prop->size, H5P_PROP_WITHIN_LIST, prop->value, prop->create, prop->set, prop->get, prop->encode, prop->decode, @@ -4939,14 +4932,14 @@ H5P__copy_prop_plist(hid_t dst_id, hid_t src_id, const char *name) if ((new_prop->create)(new_prop->name, new_prop->size, new_prop->value) < 0) HGOTO_ERROR(H5E_PLIST, H5E_CANTINIT, FAIL, "Can't initialize property"); } /* end if */ + } /* end else */ - /* Insert property into property list class */ - if (H5P__add_prop(dst_plist->props, new_prop) < 0) - HGOTO_ERROR(H5E_PLIST, H5E_CANTINSERT, FAIL, "Can't insert property into class"); + /* Insert the initialized property into the property list */ + if (H5P__add_prop(dst_plist->props, new_prop) < 0) + HGOTO_ERROR(H5E_PLIST, H5E_CANTINSERT, FAIL, "Can't insert property into list"); - /* Increment property count for class */ - dst_plist->nprops++; - } /* end else */ + /* Increment the number of properties in list */ + dst_plist->nprops++; done: /* Cleanup, if necessary */ From ede5d497fdaa9048466cf9081c6910b868d2af33 Mon Sep 17 00:00:00 2001 From: Matthew Larson Date: Thu, 6 Jun 2024 13:05:22 -0500 Subject: [PATCH 2/2] update H5Pcopy_prop comment --- src/H5P.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/H5P.c b/src/H5P.c index b69ffd6de33..a6514af265b 100644 --- a/src/H5P.c +++ b/src/H5P.c @@ -1331,8 +1331,8 @@ H5Premove(hid_t plist_id, const char *name) property information will be copied from the source class into the destination class. - If a property is copied from one list to another, the property will be - first deleted from the destination list (generating a call to the 'close' + If a property is copied from one list to another list that already contains the property, + the property will be first deleted from the destination list (generating a call to the 'del' callback for the property, if one exists) and then the property is copied from the source list to the destination list (generating a call to the 'copy' callback for the property, if one exists).