From d553c60eb0428adaa3b1d9c647a1b465b65b8408 Mon Sep 17 00:00:00 2001 From: Elena Pourmal Date: Tue, 5 Sep 2006 18:36:01 -0500 Subject: [svn-r12650] Description: Purify found some memory leaks in the code related to the HDF5 external links. James provided the fix and asked me to check it in. Tested: heping, mir, shanti, and juniper --- src/H5Glink.c | 13 +++++++++---- src/H5Gstab.c | 13 +++++++++---- src/H5Gtraverse.c | 4 ++++ src/H5L.c | 31 ++++++++++++++++++++++++++++++- src/H5Olink.c | 12 ++++++++---- 5 files changed, 60 insertions(+), 13 deletions(-) diff --git a/src/H5Glink.c b/src/H5Glink.c index 1139e7c..018088e 100644 --- a/src/H5Glink.c +++ b/src/H5Glink.c @@ -334,10 +334,15 @@ H5G_link_convert(H5O_link_t *lnk, const H5G_entry_t *ent, const H5HL_t *heap, s = H5HL_offset_into(ent->file, heap, ent->cache.ulink.udata_offset); HDassert(s); - /* Read udata from heap */ - if(NULL== (lnk->u.ud.udata = H5MM_malloc(lnk->u.ud.size))) - HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate memory for user link data") - HDmemcpy(lnk->u.ud.udata, s, lnk->u.ud.size); + /* Read udata from heap if it exists */ + if(lnk->u.ud.size > 0) + { + if(NULL== (lnk->u.ud.udata = H5MM_malloc(lnk->u.ud.size))) + HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "unable to allocate memory for user link data") + HDmemcpy(lnk->u.ud.udata, s, lnk->u.ud.size); + } + else + lnk->u.ud.udata = NULL; } break; diff --git a/src/H5Gstab.c b/src/H5Gstab.c index 926aa0e..5f52b16 100644 --- a/src/H5Gstab.c +++ b/src/H5Gstab.c @@ -646,19 +646,24 @@ H5G_stab_lookup_cb(const H5G_entry_t *ent, void *_udata) data_size =ent->cache.ulink.udata_size; + /* If there is user data, allocate space for it and copy it from the heap */ if(data_size > 0) { s = H5HL_offset_into(udata->file, heap, ent->cache.ulink.udata_offset); - /* Allocate space for the user data and copy it from the heap */ - udata->lnk->u.ud.udata = H5MM_malloc(data_size); - HDmemcpy(udata->lnk->u.ud.udata, s, data_size); + udata->lnk->u.ud.udata = H5MM_malloc(data_size); + HDmemcpy(udata->lnk->u.ud.udata, s, data_size); } /* end if */ + else + udata->lnk->u.ud.udata = NULL; /* Release the local heap */ if(H5HL_unprotect(udata->file, udata->dxpl_id, heap, udata->heap_addr, H5AC__NO_FLAGS_SET) < 0) + { + /* Release allocated memory before exiting */ + H5MM_free(udata->lnk->u.ud.udata); HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "unable to read unprotect link value") - + } /* Set link size and type */ udata->lnk->u.ud.size = data_size; udata->lnk->type = ent->cache.ulink.link_type; diff --git a/src/H5Gtraverse.c b/src/H5Gtraverse.c index 4128604..acdd665 100644 --- a/src/H5Gtraverse.c +++ b/src/H5Gtraverse.c @@ -555,6 +555,8 @@ H5G_traverse_real(const H5G_loc_t *_loc, const char *name, unsigned target, /* Free information for link (but don't free link pointer) */ if(lnk.type == H5L_LINK_SOFT) lnk.u.soft.name = H5MM_xfree(lnk.u.soft.name); + else if(lnk.type >= H5L_LINK_UD_MIN && lnk.u.ud.size > 0) + lnk.u.ud.udata = H5MM_xfree(lnk.u.ud.udata); lnk.name = H5MM_xfree(lnk.name); #endif /* H5_GROUP_REVISION */ link_valid = FALSE; @@ -751,6 +753,8 @@ done: /* Free information for link (but don't free link pointer) */ if(lnk.type == H5L_LINK_SOFT) lnk.u.soft.name = H5MM_xfree(lnk.u.soft.name); + else if(lnk.type >= H5L_LINK_UD_MIN && lnk.u.ud.size > 0) + lnk.u.ud.udata = H5MM_xfree(lnk.u.ud.udata); lnk.name = H5MM_xfree(lnk.name); #endif /* H5_GROUP_REVISION */ } /* end if */ diff --git a/src/H5L.c b/src/H5L.c index 59542da..5ce9bc5 100644 --- a/src/H5L.c +++ b/src/H5L.c @@ -1441,6 +1441,9 @@ H5L_create_ud(H5G_loc_t *link_loc, const char *link_name, void * ud_data, HDassert(ud_data_size >= 0); HDassert(ud_data_size == 0 || ud_data); + /* Initialize the link struct's pointer to its udata buffer */ + lnk.u.ud.udata = NULL; + /* Make sure that this link class is registered */ if(H5L_find_class_idx(type) < 0) HGOTO_ERROR(H5E_ARGS, H5E_BADTYPE, FAIL, "link class has not been registered with library") @@ -1463,6 +1466,9 @@ H5L_create_ud(H5G_loc_t *link_loc, const char *link_name, void * ud_data, HGOTO_ERROR(H5E_LINK, H5E_CANTINIT, FAIL, "unable to register new name for object") done: + /* Free the link's udata buffer if it's been allocated */ + H5MM_xfree(lnk.u.ud.udata); + FUNC_LEAVE_NOAPI(ret_value) } /* end H5L_create_ud() */ @@ -1740,7 +1746,8 @@ H5L_move_dest_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *l /* Give the object its new name */ /* Casting away const okay -JML */ - udata->lnk->name = H5MM_xfree(udata->lnk->name); + HDassert(udata->lnk->name == NULL); +/* JAMES udata->lnk->name = H5MM_xfree(udata->lnk->name); */ udata->lnk->name= (char *)name; /* Insert the link into the group */ @@ -1837,6 +1844,7 @@ H5L_move_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, H5G_obj_t type; /* Type of object being moved */ H5RS_str_t *dst_name_r = NULL; /* Ref-counted version of dest name */ char * orig_name = NULL; /* The name of the link in this group */ + hbool_t link_copied = FALSE; /* Has udata_out.lnk been allocated? */ herr_t ret_value = SUCCEED; /* Return value */ FUNC_ENTER_NOAPI_NOINIT(H5L_move_cb) @@ -1869,6 +1877,12 @@ H5L_move_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, /* Set up user data for move_dest_cb */ if((udata_out.lnk = H5O_link_copy(lnk, NULL, 0)) == NULL) HGOTO_ERROR(H5E_LINK, H5E_CANTCOPY, FAIL, "unable to copy link to be moved"); + /* In this special case, the link's name is going to be replaced at its + * destination, so we should free it here. + */ + udata_out.lnk->name = H5MM_xfree(udata_out.lnk->name); + link_copied = TRUE; + udata_out.lnk->cset = udata->cset; udata_out.file = grp_loc->oloc->file; udata_out.copy = udata->copy; @@ -1893,6 +1907,7 @@ H5L_move_cb(H5G_loc_t *grp_loc/*in*/, const char *name, const H5O_link_t *lnk, /* Remove the old link */ if(H5G_obj_remove(grp_loc->oloc, orig_name, &type, udata->dxpl_id) < 0) HGOTO_ERROR(H5E_SYM, H5E_NOTFOUND, FAIL, "unable to remove old name") + H5RS_decr(dst_name_r); } done: @@ -1900,6 +1915,20 @@ done: if(orig_name) H5MM_xfree(orig_name); + /* If udata_out.lnk was copied, free any memory allocated + * In this special case, the H5L_move_dest_cb callback frees the name + * if it succeeds + */ + if(link_copied) + { + if(udata_out.lnk->type == H5L_LINK_SOFT) + udata_out.lnk->u.soft.name = H5MM_xfree(udata_out.lnk->u.soft.name); + else if(udata_out.lnk->type >= H5L_LINK_UD_MIN && udata_out.lnk->u.ud.size > 0) + udata_out.lnk->u.ud.udata = H5MM_xfree(udata_out.lnk->u.ud.udata); + /* JAMES: the dest_cb already frees the link name. Hmm. */ + H5MM_xfree(udata_out.lnk); + } + /* Indicate that this callback didn't take ownership of the group * * location for the object */ *own_loc = H5G_OWN_NONE; diff --git a/src/H5Olink.c b/src/H5Olink.c index 53fa99e..039184d 100644 --- a/src/H5Olink.c +++ b/src/H5Olink.c @@ -165,7 +165,7 @@ H5O_link_decode(H5F_t *f, hid_t UNUSED dxpl_id, const uint8_t *p) p += len; break; - /* User-defined linkes */ + /* User-defined links */ default: if(lnk->type < H5L_LINK_UD_MIN || lnk->type > H5L_LINK_MAX) HGOTO_ERROR(H5E_OHDR, H5E_CANTLOAD, NULL, "unknown link type") @@ -180,6 +180,8 @@ H5O_link_decode(H5F_t *f, hid_t UNUSED dxpl_id, const uint8_t *p) HDmemcpy(lnk->u.ud.udata, p, len); p += len; } + else + lnk->u.ud.udata = NULL; } /* end switch */ /* Set return value */ @@ -192,6 +194,8 @@ done: H5MM_xfree(lnk->name); if(lnk->type == H5L_LINK_SOFT && lnk->u.soft.name != NULL) H5MM_xfree(lnk->u.soft.name); + if(lnk->type >= H5L_LINK_UD_MIN && lnk->u.ud.size > 0 && lnk->u.ud.udata != NULL) + H5MM_xfree(lnk->u.ud.udata); H5FL_FREE(H5O_link_t, lnk); } /* end if */ @@ -323,7 +327,7 @@ H5O_link_copy(const void *_mesg, void *_dest, unsigned UNUSED update_flags) if(lnk->type == H5L_LINK_SOFT) dest->u.soft.name = H5MM_xstrdup(lnk->u.soft.name); else if(lnk->type >= H5L_LINK_UD_MIN) { - if(lnk->u.ud.size != 0) + if(lnk->u.ud.size > 0) { if(NULL == (dest->u.ud.udata = H5MM_malloc(lnk->u.ud.size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") @@ -601,8 +605,8 @@ H5O_link_copy_file(H5F_t UNUSED *file_src, void *native_src, H5F_t UNUSED *file_ default: if(link_src->type >= H5L_LINK_UD_MIN) { - /* Copy the user-defined link's user data */ - if(link_src->u.ud.size != 0) + /* Copy the user-defined link's user data if it exists */ + if(link_src->u.ud.size > 0) { if(NULL == (link_dst->u.ud.udata = H5MM_malloc(link_src->u.ud.size))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed") -- cgit v0.12