[svn-r9552] Purpose:

    Code optimization

Description:
    Avoid making as many copies of attribute information.

    Also, be smarter about which properties we've seen before when copying
and closing property lists.

    Fix memory leak of attribute data structures.

Platforms tested:
    FreeBSD 4.10 (sleipnir) w/parallel
    Solaris 2.7 (arabica)
    Too minor to require h5committest

3 files changed, 76 insertions, 38 deletions

diff --git a/src/H5A.c b/src/H5A.c
index e264026..1874187 100644
--- a/src/H5A.c
+++ b/src/H5A.c
@@ -362,8 +362,7 @@ H5A_get_index(H5G_entry_t *ent, const char *name, hid_t dxpl_id)
 	if(HDstrcmp(found_attr.name,name)==0) {
 	    if(H5O_reset (H5O_ATTR_ID, &found_attr)<0)
                 HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info")
-	    ret_value = i;
-	    break;
+            HGOTO_DONE(i);
 	}
 	if(H5O_reset (H5O_ATTR_ID, &found_attr)<0)
             HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info")
@@ -522,7 +521,7 @@ H5A_open(H5G_entry_t *ent, unsigned idx, hid_t dxpl_id)
 
     /* Read in attribute with H5O_read() */
     H5_CHECK_OVERFLOW(idx,unsigned,int);
-    if (NULL==(attr=H5O_read(ent, H5O_ATTR_ID, (int)idx, attr, dxpl_id)))
+    if (NULL==(attr=H5O_read(ent, H5O_ATTR_ID, (int)idx, NULL, dxpl_id)))
         HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to load attribute info from dataset header")
     attr->initialized=1;
    
@@ -1228,7 +1227,7 @@ static herr_t
 H5A_rename(H5G_entry_t *ent, const char *old_name, const char *new_name, hid_t dxpl_id)
 {
     int         seq, idx=FAIL;  /* Index of attribute being querried */
-    H5A_t       *found_attr;    /* Attribute with OLD_NAME */
+    H5A_t       found_attr;     /* Attribute with OLD_NAME */
     herr_t	ret_value=SUCCEED;      /* Return value */
     
     FUNC_ENTER_NOAPI_NOINIT(H5A_rename)
@@ -1238,45 +1237,41 @@ H5A_rename(H5G_entry_t *ent, const char *old_name, const char *new_name, hid_t d
     assert(old_name);
     assert(new_name);
     
-    /* Build the attribute information */
-    if((found_attr = H5FL_CALLOC(H5A_t))==NULL)
-        HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for attribute info")
-        
     /* Read in the existing attributes to check for duplicates */
     seq=0;
-    while(H5O_read(ent, H5O_ATTR_ID, seq, found_attr, dxpl_id)!=NULL) {
+    while(H5O_read(ent, H5O_ATTR_ID, seq, &found_attr, dxpl_id)!=NULL) {
         /*
 	 * Compare found attribute name.
 	 */
-	if(HDstrcmp(found_attr->name,old_name)==0) {
+	if(HDstrcmp(found_attr.name,old_name)==0) {
             idx = seq;
             break;
 	}
-	if(H5O_reset (H5O_ATTR_ID, found_attr)<0)
+	if(H5O_reset (H5O_ATTR_ID, &found_attr)<0)
 	    HGOTO_ERROR(H5E_ATTR, H5E_CANTFREE, FAIL, "can't release attribute info")
 	seq++;
     }
- 
+    H5E_clear_stack(NULL);
     if(idx<0)
 	HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "attribute cannot be found")
         
     /* Copy the attribute name. */
-    if(found_attr->name)
-        HDfree(found_attr->name);
-    found_attr->name = HDstrdup(new_name); 
-    if(!found_attr->name) 
+    if(found_attr.name)
+        HDfree(found_attr.name);
+    found_attr.name = HDstrdup(new_name); 
+    if(!found_attr.name) 
         HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "String copy failed")
 
     /* Indicate entry is not opened and the attribute doesn't need fill-values. */
-    found_attr->ent_opened=FALSE;
-    found_attr->initialized=TRUE;
+    found_attr.ent_opened=FALSE;
+    found_attr.initialized=TRUE;
 
     /* Modify the attribute message */
-    if (H5O_modify(ent, H5O_ATTR_ID, idx, 0, 1, found_attr, dxpl_id) < 0) 
+    if (H5O_modify(ent, H5O_ATTR_ID, idx, 0, 1, &found_attr, dxpl_id) < 0) 
         HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, FAIL, "unable to update attribute header messages")
    
     /* Close the attribute */
-    if(H5A_close(found_attr)<0)
+    if(H5A_free(&found_attr)<0)
         HGOTO_ERROR(H5E_ATTR, H5E_CANTRELEASE, FAIL, "unable to close renamed attribute")
 
 done:
diff --git a/src/H5Oattr.c b/src/H5Oattr.c
index 79262b4..9f92c87 100644
--- a/src/H5Oattr.c
+++ b/src/H5Oattr.c
@@ -35,6 +35,7 @@ static void *H5O_attr_decode (H5F_t *f, hid_t dxpl_id, const uint8_t *p, H5O_sha
 static void *H5O_attr_copy (const void *_mesg, void *_dest);
 static size_t H5O_attr_size (H5F_t *f, const void *_mesg);
 static herr_t H5O_attr_reset (void *_mesg);
+static herr_t H5O_attr_free (void *mesg);
 static herr_t H5O_attr_delete (H5F_t *f, hid_t dxpl_id, const void *_mesg);
 static herr_t H5O_attr_link(H5F_t *f, hid_t dxpl_id, const void *_mesg);
 static herr_t H5O_attr_debug (H5F_t *f, hid_t dxpl_id, const void *_mesg,
@@ -50,7 +51,7 @@ const H5O_class_t H5O_ATTR[1] = {{
     H5O_attr_copy,		/* copy the native value        */
     H5O_attr_size,		/* size of raw message          */
     H5O_attr_reset,		/* reset method                 */
-    NULL,		        /* free method			*/
+    H5O_attr_free,	        /* free method			*/
     H5O_attr_delete,		/* file delete method		*/
     H5O_attr_link,		/* link method			*/
     NULL,			/* get share method		*/
@@ -371,7 +372,6 @@ static void            *
 H5O_attr_copy(const void *_src, void *_dst)
 {
     const H5A_t            *src = (const H5A_t *) _src;
-    H5A_t                  *dst = NULL;
     void                   *ret_value;  /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT(H5O_attr_copy);
@@ -380,12 +380,9 @@ H5O_attr_copy(const void *_src, void *_dst)
     assert(src);
 
     /* copy */
-    if (NULL == (dst = H5A_copy(_dst,src)))
+    if (NULL == (ret_value = H5A_copy(_dst,src)))
         HGOTO_ERROR(H5E_ATTR, H5E_CANTINIT, NULL, "can't copy attribute");
 
-    /* Set return value */
-    ret_value=dst;
-
 done:
     FUNC_LEAVE_NOAPI(ret_value);
 }
@@ -482,7 +479,6 @@ static herr_t
 H5O_attr_reset(void *_mesg)
 {
     H5A_t                  *attr = (H5A_t *) _mesg;
-    H5A_t                  *tmp = NULL;
     herr_t      ret_value=SUCCEED;       /* Return value */
 
     FUNC_ENTER_NOAPI_NOINIT(H5O_attr_reset);
@@ -490,12 +486,38 @@ H5O_attr_reset(void *_mesg)
     if (attr)
         H5A_free(attr);
 
-done:
     FUNC_LEAVE_NOAPI(ret_value);
 }
 
 
 /*-------------------------------------------------------------------------
+ * Function:	H5O_attr_free
+ *
+ * Purpose:	Free's the message
+ *
+ * Return:	Non-negative on success/Negative on failure
+ *
+ * Programmer:	Quincey Koziol
+ *              Thursday, November 18, 2004
+ *
+ * Modifications:
+ *
+ *-------------------------------------------------------------------------
+ */
+static herr_t
+H5O_attr_free (void *mesg)
+{
+    FUNC_ENTER_NOAPI_NOINIT_NOFUNC(H5O_attr_free);
+
+    assert (mesg);
+
+    H5FL_FREE(H5A_t,mesg);
+
+    FUNC_LEAVE_NOAPI(SUCCEED);
+} /* end H5O_attr_free() */
+
+
+/*-------------------------------------------------------------------------
  * Function:    H5O_attr_delete
  *
  * Purpose:     Free file space referenced by message
diff --git a/src/H5P.c b/src/H5P.c
index 7e6be49..43a6f3f 100644
--- a/src/H5P.c
+++ b/src/H5P.c
@@ -467,6 +467,8 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
     hid_t new_plist_id;         /* Property list ID of new list created */
     H5TB_NODE *curr_node;       /* Current node in TBBT */
     H5TB_TREE *seen=NULL;       /* TBBT containing properties already seen */
+    size_t nseen;               /* Number of items 'seen' */
+    hbool_t has_parent_class;   /* Flag to indicate that this property list's class has a parent */
     hid_t ret_value=FAIL;       /* return value */
  
     FUNC_ENTER_NOAPI(H5P_copy_plist, FAIL);
@@ -501,6 +503,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
      */
     if((seen=H5TB_fast_dmake(H5TB_FAST_STR_COMPARE))==NULL)
         HGOTO_ERROR(H5E_PLIST,H5E_CANTMAKETREE,FAIL,"can't create TBBT for seen properties");
+    nseen=0;
 
     /* Cycle through the deleted properties & copy them into the new list's deleted section */
     if(old_plist->del->root) {
@@ -519,6 +522,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
             /* Add property name to "seen" list */
             if(H5TB_dins(seen,new_name,new_name)==NULL)
                 HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+            nseen++;
 
             /* Get the next property node in the TBBT */
             curr_node=H5TB_next(curr_node);
@@ -553,6 +557,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
             /* Add property name to "seen" list */
             if(H5TB_dins(seen,new_prop->name,new_prop->name)==NULL)
                 HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+            nseen++;
 
             /* Increment the number of properties in list */
             new_plist->nprops++;
@@ -567,6 +572,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
      * initialize each with default value & make property 'copy' callback.
      */
     tclass=old_plist->pclass;
+    has_parent_class=(tclass!=NULL && tclass->parent!=NULL && tclass->parent->nprops>0);
     while(tclass!=NULL) {
         if(tclass->nprops>0) {
             /* Walk through the properties in the old class */
@@ -576,7 +582,7 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
                 tmp=curr_node->data;
 
                 /* Only "copy" properties we haven't seen before */
-                if(H5TB_dfind(seen,tmp->name,NULL)==NULL) {
+                if(nseen==0 || H5TB_dfind(seen,tmp->name,NULL)==NULL) {
                     /* Call property creation callback, if it exists */
                     if(tmp->copy) {
                         /* Call the callback & insert changed value into tree (if necessary) */
@@ -584,9 +590,12 @@ H5P_copy_plist(H5P_genplist_t *old_plist)
                             HGOTO_ERROR (H5E_PLIST, H5E_CANTCOPY, FAIL,"Can't create property");
                     } /* end if */
 
-                    /* Add property name to "seen" list */
-                    if(H5TB_dins(seen,tmp->name,tmp->name)==NULL)
-                        HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+                    /* Add property name to "seen" list, if we have other classes to work on */
+                    if(has_parent_class) {
+                        if(H5TB_dins(seen,tmp->name,tmp->name)==NULL)
+                            HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+                        nseen++;
+                    } /* end if */
 
                     /* Increment the number of properties in list */
                     new_plist->nprops++;
@@ -4454,7 +4463,6 @@ H5P_get(H5P_genplist_t *plist, const char *name, void *value)
 
     assert(plist);
     assert(name);
-
     assert(value);
 
     /* Check if the property has been deleted */
@@ -5188,6 +5196,9 @@ H5P_close(void *_plist)
     H5P_genclass_t *tclass;         /* Temporary class pointer */
     H5P_genplist_t *plist=(H5P_genplist_t *)_plist;
     H5TB_TREE *seen=NULL;           /* TBBT to hold names of properties already seen */
+    size_t nseen;                   /* Number of items 'seen' */
+    hbool_t has_parent_class;       /* Flag to indicate that this property list's class has a parent */
+    ssize_t ndel;                   /* Number of items deleted */
     H5TB_NODE *curr_node;           /* Current node in TBBT */
     H5P_genprop_t *tmp;             /* Temporary pointer to properties */
     herr_t ret_value=SUCCEED;       /* return value */
@@ -5209,6 +5220,7 @@ H5P_close(void *_plist)
      */
     if((seen=H5TB_fast_dmake(H5TB_FAST_STR_COMPARE))==NULL)
         HGOTO_ERROR(H5E_PLIST,H5E_CANTMAKETREE,FAIL,"can't create TBBT for seen properties");
+    nseen=0;
 
     /* Walk through the changed properties in the list */
     if(plist->props->root) {
@@ -5226,17 +5238,23 @@ H5P_close(void *_plist)
             /* Add property name to "seen" list */
             if(H5TB_dins(seen,tmp->name,tmp->name)==NULL)
                 HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+            nseen++;
 
             /* Get the next property node in the TBBT */
             curr_node=H5TB_next(curr_node);
         } /* end while */
     } /* end if */
 
+    /* Determine number of deleted items from property list */
+    if((ndel=H5TB_count(plist->del))<0)
+        HGOTO_ERROR(H5E_PLIST,H5E_CANTCOUNT,FAIL,"can't deterimine # of items in TBBT");
+
     /*
      * Check if we should remove class properties (up through list of parent classes also),
      * initialize each with default value & make property 'remove' callback.
      */
     tclass=plist->pclass;
+    has_parent_class=(tclass!=NULL && tclass->parent!=NULL && tclass->parent->nprops>0);
     while(tclass!=NULL) {
         if(tclass->nprops>0) {
             /* Walk through the properties in the class */
@@ -5248,8 +5266,8 @@ H5P_close(void *_plist)
                 /* Only "delete" properties we haven't seen before
                  * and that haven't already been deleted
                  */
-                if(H5TB_dfind(seen,tmp->name,NULL)==NULL &&
-                        H5TB_dfind(plist->del,tmp->name,NULL)==NULL) {
+                if((nseen==0 || H5TB_dfind(seen,tmp->name,NULL)==NULL) &&
+                        (ndel==0 || H5TB_dfind(plist->del,tmp->name,NULL)==NULL)) {
 
                     /* Call property close callback, if it exists */
                     if(tmp->close) {
@@ -5267,9 +5285,12 @@ H5P_close(void *_plist)
                         H5MM_xfree(tmp_value);
                     } /* end if */
 
-                    /* Add property name to "seen" list */
-                    if(H5TB_dins(seen,tmp->name,tmp->name)==NULL)
-                        HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+                    /* Add property name to "seen" list, if we have other classes to work on */
+                    if(has_parent_class) {
+                        if(H5TB_dins(seen,tmp->name,tmp->name)==NULL)
+                            HGOTO_ERROR(H5E_PLIST,H5E_CANTINSERT,FAIL,"can't insert property into seen TBBT");
+                        nseen++;
+                    } /* end if */
                 } /* end if */
 
                 /* Get the next property node in the TBBT */