From 6ae26dba82e56fd6e895ca116a51da070470ba5b Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Sun, 8 Aug 2004 17:12:18 -0500 Subject: [svn-r9053] 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 | 68 +++++++++------ src/H5Tconv.c | 6 ++ test/dtypes.c | 215 +++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 265 insertions(+), 28 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index d4f95696..b0646b1 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -171,6 +171,10 @@ Bug Fixes since HDF5-1.6.0 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 afd4b14..cdd7d20 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -2367,36 +2367,48 @@ 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(NULL); /*ignore all shutdown errors*/ - } + } + H5T_close(path->src); + H5T_close(path->dst); + H5FL_FREE(H5T_path_t,path); + H5E_clear(NULL); /*ignore all shutdown errors*/ + } /* end else */ + } /* end for */ done: FUNC_LEAVE_NOAPI(ret_value); diff --git a/src/H5Tconv.c b/src/H5Tconv.c index a419577..0a500b5 100644 --- a/src/H5Tconv.c +++ b/src/H5Tconv.c @@ -1534,6 +1534,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 607b65a..178ae40 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -121,6 +121,11 @@ static hbool_t overflows(unsigned char *origin_bits, dtype_t src_dtype, size_t src_size_bytes, size_t dst_num_bits); static int my_isnan(dtype_t type, void *val); static int opaque_check(int tag_it); +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); /*------------------------------------------------------------------------- @@ -1860,6 +1865,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