From f9e414bc751c50eee75bc7ec77690843c68c6f9a Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Sun, 8 Aug 2004 17:12:21 -0500 Subject: [svn-r9054] Purpose: Bug fix Description: Correct possible core dump when a datatype conversion function is registered with the library after a compound datatype has been converted (having it's type conversion information cached by the library). The compound datatype must have been created by inserting the fields in non-increasing offset order to see the bug. Solution: Re-sort the fields in the compound datatypes before recalculating the cached information when performing the conversion on them. Platforms tested: FreeBSD 4.10 (sleipnir) h5committested --- release_docs/RELEASE.txt | 4 + src/H5T.c | 66 +++++++++------ src/H5Tconv.c | 6 ++ test/dtypes.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 264 insertions(+), 27 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 22b677e..0c07438 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -108,6 +108,10 @@ Bug Fixes since HDF5-1.6.2 release Library ------- + - Fixed error which could cause a core dump when a type conversion + routine was registered after a compound datatype had been + converted and then an equivalment compound datatype was converted + again. QAK - 2004/08/07 - Fixed memory overwrite when encoding "multi" file driver information for file's superblock. QAK - 2004/08/05 - Fixed obscure bug where a filter which failed during chunk allocation diff --git a/src/H5T.c b/src/H5T.c index dcbad77..3bcd0f0 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -2304,35 +2304,47 @@ H5T_unregister(H5T_pers_t pers, const char *name, H5T_t *src, H5T_t *dst, for (i=H5T_g.npaths-1; i>0; --i) { path = H5T_g.path[i]; assert(path); - if ((H5T_PERS_SOFT==pers && path->is_hard) || - (H5T_PERS_HARD==pers && !path->is_hard)) continue; - if (name && *name && HDstrcmp(name, path->name)) continue; - if (src && H5T_cmp(src, path->src)) continue; - if (dst && H5T_cmp(dst, path->dst)) continue; - if (func && func!=path->func) continue; - - /* Remove from table */ - HDmemmove(H5T_g.path+i, H5T_g.path+i+1, - (H5T_g.npaths-(i+1))*sizeof(H5T_path_t*)); - --H5T_g.npaths; - - /* Shut down path */ - H5T_print_stats(path, &nprint); - path->cdata.command = H5T_CONV_FREE; - if ((path->func)(FAIL, FAIL, &(path->cdata), 0, 0, 0, NULL, NULL, - dxpl_id)<0) { + + /* Not a match */ + if (((H5T_PERS_SOFT==pers && path->is_hard) || + (H5T_PERS_HARD==pers && !path->is_hard)) || + (name && *name && HDstrcmp(name, path->name)) || + (src && H5T_cmp(src, path->src)) || + (dst && H5T_cmp(dst, path->dst)) || + (func && func!=path->func)) { + /* + * Notify all other functions to recalculate private data since some + * functions might cache a list of conversion functions. For + * instance, the compound type converter caches a list of conversion + * functions for the members, so removing a function should cause + * the list to be recalculated to avoid the removed function. + */ + path->cdata.recalc = TRUE; + } /* end if */ + else { + /* Remove from table */ + HDmemmove(H5T_g.path+i, H5T_g.path+i+1, + (H5T_g.npaths-(i+1))*sizeof(H5T_path_t*)); + --H5T_g.npaths; + + /* Shut down path */ + H5T_print_stats(path, &nprint); + path->cdata.command = H5T_CONV_FREE; + if ((path->func)(FAIL, FAIL, &(path->cdata), 0, 0, 0, NULL, NULL, + dxpl_id)<0) { #ifdef H5T_DEBUG - if (H5DEBUG(T)) { - fprintf(H5DEBUG(T), "H5T: conversion function 0x%08lx failed " - "to free private data for %s (ignored)\n", - (unsigned long)(path->func), path->name); - } + if (H5DEBUG(T)) { + fprintf(H5DEBUG(T), "H5T: conversion function 0x%08lx failed " + "to free private data for %s (ignored)\n", + (unsigned long)(path->func), path->name); + } #endif - } - H5T_close(path->src); - H5T_close(path->dst); - H5FL_FREE(H5T_path_t,path); - H5E_clear(); /*ignore all shutdown errors*/ + } + H5T_close(path->src); + H5T_close(path->dst); + H5FL_FREE(H5T_path_t,path); + H5E_clear(); /*ignore all shutdown errors*/ + } /* end else */ } done: diff --git a/src/H5Tconv.c b/src/H5Tconv.c index fd7693a..2bce779 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -1363,6 +1363,12 @@ H5T_conv_struct_init (H5T_t *src, H5T_t *dst, H5T_cdata_t *cdata, hid_t dxpl_id) } } } + else { + /* Restore sorted conditions for the datatypes */ + /* (Required for the src2dst array to be valid) */ + H5T_sort_value(src, NULL); + H5T_sort_value(dst, NULL); + } /* end else */ /* * (Re)build the cache of member conversion functions and pointers to diff --git a/test/dtypes.c b/test/dtypes.c index 24f78a8..00001f3 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -123,6 +123,11 @@ void some_dummy_func(float x); static int opaque_check(int tag_it); void *test_vltypes_alloc_custom(size_t size, void *info); void test_vltypes_free_custom(void *mem, void *info); +static herr_t +convert_opaque(hid_t UNUSED st, hid_t UNUSED dt, H5T_cdata_t *cdata, + size_t UNUSED nelmts, size_t UNUSED buf_stride, + size_t UNUSED bkg_stride, void UNUSED *_buf, + void UNUSED *bkg, hid_t UNUSED dset_xfer_plid); /*------------------------------------------------------------------------- @@ -1824,6 +1829,211 @@ test_compound_10(void) /*------------------------------------------------------------------------- + * Function: test_compound_11 + * + * Purpose: Tests whether registering/unregistering a type conversion + * function correctly causes compound datatypes to recalculate + * their cached field conversion information + * + * Return: Success: 0 + * + * Failure: number of errors + * + * Programmer: Quincey Koziol + * Saturday, August 7, 2004 + * + * Modifications: + * + *------------------------------------------------------------------------- + */ +static int +test_compound_11(void) +{ + typedef struct { + double d1; + int i1; + char *s1; + int i2; + double d2; + double d3; + } big_t; + + typedef struct { + double d1; + int i1; + char *s1; + } little_t; + + hid_t var_string_tid; /* Datatype IDs for type conversion */ + hid_t big_tid, little_tid; /* Datatype IDs for type conversion */ + hid_t big_tid2, little_tid2; /* Datatype IDs for type conversion */ + hid_t opaq_src_tid, opaq_dst_tid; /* Datatype IDs for type conversion */ + void *buf, /* Conversion buffer */ + *buf_orig, /* Copy of original conversion buffer */ + *bkg; /* Background buffer */ + size_t u; /* Local index variable */ + + TESTING("registering type conversion routine with compound conversions"); + + /* Create variable string type for use in both structs */ + if((var_string_tid=H5Tcopy(H5T_C_S1))<0) TEST_ERROR + if(H5Tset_size(var_string_tid,H5T_VARIABLE)<0) TEST_ERROR + + /* Create type for 'big' struct */ + if((big_tid = H5Tcreate(H5T_COMPOUND, sizeof(big_t)))<0) TEST_ERROR + + if(H5Tinsert(big_tid, "d1", HOFFSET(big_t, d1), H5T_NATIVE_DOUBLE)<0) TEST_ERROR + if(H5Tinsert(big_tid, "i1", HOFFSET(big_t, i1), H5T_NATIVE_INT)<0) TEST_ERROR + if(H5Tinsert(big_tid, "s1", HOFFSET(big_t, s1), var_string_tid)<0) TEST_ERROR + if(H5Tinsert(big_tid, "i2", HOFFSET(big_t, i2), H5T_NATIVE_INT)<0) TEST_ERROR + if(H5Tinsert(big_tid, "d2", HOFFSET(big_t, d2), H5T_NATIVE_DOUBLE)<0) TEST_ERROR + if(H5Tinsert(big_tid, "d3", HOFFSET(big_t, d3), H5T_NATIVE_DOUBLE)<0) TEST_ERROR + + /* Create type for 'little' struct (with "out of order" inserts) */ + if((little_tid = H5Tcreate(H5T_COMPOUND, sizeof(little_t)))<0) TEST_ERROR + + if(H5Tinsert(little_tid, "d1", HOFFSET(little_t, d1), H5T_NATIVE_DOUBLE)<0) TEST_ERROR + if(H5Tinsert(little_tid, "s1", HOFFSET(little_t, s1), var_string_tid)<0) TEST_ERROR + if(H5Tinsert(little_tid, "i1", HOFFSET(little_t, i1), H5T_NATIVE_INT)<0) TEST_ERROR + + /* Allocate buffers */ + if((buf=HDmalloc(sizeof(big_t)*NTESTELEM))==NULL) TEST_ERROR + if((buf_orig=HDmalloc(sizeof(big_t)*NTESTELEM))==NULL) TEST_ERROR + if((bkg=HDmalloc(sizeof(big_t)*NTESTELEM))==NULL) TEST_ERROR + + /* Initialize buffer */ + for(u=0; u