From 89f283e2e47814419e7e6861bfa76f56e27124d2 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Sat, 23 Jul 2005 14:58:23 -0500 Subject: [svn-r11146] Purpose: Bug fix Description: If a named datatype is copied and the copy is used to create a dataset, the dataset would inadvertantly refer to the original named datatype instead of a local (possibly modified) copy of the named datatype. Solution: Fixed datatype copying routine. Platforms tested: FreeBSD 4.11 (sleipnir) Too minor to require h5committest --- release_docs/RELEASE.txt | 3 ++ src/H5Odtype.c | 4 +- src/H5T.c | 42 ++++++++++------- test/dtypes.c | 114 ++++++++++++++++++++++++++++++----------------- 4 files changed, 105 insertions(+), 58 deletions(-) diff --git a/release_docs/RELEASE.txt b/release_docs/RELEASE.txt index 982345c..6cdf888 100644 --- a/release_docs/RELEASE.txt +++ b/release_docs/RELEASE.txt @@ -72,6 +72,9 @@ Bug Fixes since HDF5-1.6.4 release Library ------- + - Fixed a bug with named datatypes where a copy of a named datatype + used to create a dataset would accidentally use the original + named datatype for the dataset's datatype. QAK - 2005/07/23 - Made H5Fget_name() be consistent and always return name of actual file the ID is in. (Instead of the name of the top file in a file mounting hierarchy). QAK - 2005/07/19 diff --git a/src/H5Odtype.c b/src/H5Odtype.c index ed044c4..bf3cd1e 100644 --- a/src/H5Odtype.c +++ b/src/H5Odtype.c @@ -1131,8 +1131,8 @@ H5O_dtype_get_share(H5F_t UNUSED *f, const void *_mesg, assert (sh); if (H5F_addr_defined (dt->ent.header)) { - if(H5T_STATE_NAMED!=dt->shared->state && H5T_STATE_OPEN!=dt->shared->state && H5T_STATE_TRANSIENT!=dt->shared->state) - HGOTO_ERROR (H5E_ARGS, H5E_BADTYPE, FAIL, "datatype state is not valid"); + /* If the address is defined, this had better be a named datatype */ + HDassert (H5T_STATE_NAMED==dt->shared->state || H5T_STATE_OPEN==dt->shared->state); sh->in_gh = FALSE; sh->u.ent = dt->ent; diff --git a/src/H5T.c b/src/H5T.c index a1c6c65..215dce7 100644 --- a/src/H5T.c +++ b/src/H5T.c @@ -429,10 +429,11 @@ static herr_t H5T_register(H5T_pers_t pers, const char *name, H5T_t *src, /* Allocate new datatype info */ \ if (NULL==(dt = H5FL_CALLOC(H5T_t))) \ HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") \ - if (NULL==(dt->shared = H5FL_CALLOC(H5T_shared_t))) { \ - H5FL_FREE(H5T_t, dt); \ + dt->ent.header = HADDR_UNDEF; \ + if (NULL==(dt->shared = H5FL_CALLOC(H5T_shared_t))) { \ + H5FL_FREE(H5T_t, dt); \ HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed") \ - } \ + } \ } @@ -2630,6 +2631,7 @@ H5T_create(H5T_class_t type, size_t size) case H5T_COMPOUND: if (NULL==(dt = H5FL_CALLOC(H5T_t))) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); + dt->ent.header = HADDR_UNDEF; if (NULL==(dt->shared = H5FL_CALLOC(H5T_shared_t))) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); dt->shared->type = type; @@ -2658,6 +2660,7 @@ H5T_create(H5T_class_t type, size_t size) } if (NULL==(dt = H5FL_CALLOC(H5T_t))) HGOTO_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); + dt->ent.header = HADDR_UNDEF; if (NULL==(dt->shared = H5FL_CALLOC(H5T_shared_t))) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); dt->shared->type = type; @@ -2911,8 +2914,7 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) if (NULL==(new_dt->shared = H5FL_MALLOC(H5T_shared_t))) HGOTO_ERROR (H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed"); - /* Copy actual information */ - new_dt->ent = old_dt->ent; + /* Copy shared information (entry information is copied last) */ *(new_dt->shared) = *(old_dt->shared); /* Copy parent information */ @@ -2926,8 +2928,6 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) * Return an unlocked transient type. */ new_dt->shared->state = H5T_STATE_TRANSIENT; - HDmemset (&(new_dt->ent), 0, sizeof(new_dt->ent)); - new_dt->ent.header = HADDR_UNDEF; break; case H5T_COPY_ALL: @@ -2935,9 +2935,9 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) * Return a transient type (locked or unlocked) or an unopened named * type. Immutable transient types are degraded to read-only. */ - if (H5T_STATE_OPEN==new_dt->shared->state) { + if (H5T_STATE_OPEN==old_dt->shared->state) { new_dt->shared->state = H5T_STATE_NAMED; - } else if (H5T_STATE_IMMUTABLE==new_dt->shared->state) { + } else if (H5T_STATE_IMMUTABLE==old_dt->shared->state) { new_dt->shared->state = H5T_STATE_RDONLY; } break; @@ -2947,15 +2947,20 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) * Return a transient type (locked or unlocked) or an opened named * type. Immutable transient types are degraded to read-only. */ - if (H5F_addr_defined(new_dt->ent.header)) { + if (H5F_addr_defined(old_dt->ent.header)) { /* Check if the object is already open */ - if((reopened_fo=H5FO_opened(new_dt->ent.file,new_dt->ent.header))==NULL) { + if((reopened_fo=H5FO_opened(old_dt->ent.file, old_dt->ent.header))==NULL) { /* Clear any errors from H5FO_opened() */ H5E_clear(); - if (H5O_open (&(new_dt->ent))<0) + + /* Open named datatype again */ + if (H5O_open (&(old_dt->ent))<0) HGOTO_ERROR (H5E_DATATYPE, H5E_CANTOPENOBJ, NULL, "unable to reopen named data type"); - if(H5FO_insert(new_dt->ent.file, new_dt->ent.header,new_dt->shared)<0) + + /* Insert opened named datatype into opened object list for the file */ + if(H5FO_insert(old_dt->ent.file, old_dt->ent.header, new_dt->shared)<0) HGOTO_ERROR(H5E_DATATYPE, H5E_CANTINSERT, NULL, "can't insert datatype into list of open objects") + new_dt->shared->fo_count=1; } else { /* The object is already open. Free the H5T_shared_t struct @@ -2966,7 +2971,7 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) reopened_fo->fo_count++; } new_dt->shared->state = H5T_STATE_OPEN; - } else if (H5T_STATE_IMMUTABLE==new_dt->shared->state) { + } else if (H5T_STATE_IMMUTABLE==old_dt->shared->state) { new_dt->shared->state = H5T_STATE_RDONLY; } break; @@ -3078,9 +3083,16 @@ H5T_copy(const H5T_t *old_dt, H5T_copy_t method) } /* end switch */ /* Deep copy of the symbol table entry, if there was one */ - if (H5F_addr_defined(old_dt->ent.header)) + if ( new_dt->shared->state == H5T_STATE_NAMED || new_dt->shared->state == H5T_STATE_OPEN) { + if (!H5F_addr_defined(old_dt->ent.header)) + HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, NULL, "named dataype with invalid address"); if (H5G_ent_copy(&(new_dt->ent), &(old_dt->ent),H5G_COPY_DEEP)<0) HGOTO_ERROR(H5E_SYM, H5E_CANTOPENOBJ, NULL, "unable to copy entry"); + } /* end if */ + else { + HDmemset (&(new_dt->ent), 0, sizeof(new_dt->ent)); + new_dt->ent.header = HADDR_UNDEF; + } /* end else */ /* Set return value */ ret_value=new_dt; diff --git a/test/dtypes.c b/test/dtypes.c index 7d870df..6fccd25 100644 --- a/test/dtypes.c +++ b/test/dtypes.c @@ -342,8 +342,8 @@ test_classes(void) hid_t vls_id; /* VL string */ hid_t memb_id; /* Compound member datatype */ H5T_class_t memb_cls; - H5T_class_t tcls; - int nmembs, i; + H5T_class_t tcls; + unsigned int nmembs, i; TESTING("H5Tget_class()"); @@ -723,9 +723,9 @@ test_compound_2(void) TESTING("compound element reordering"); /* Sizes should be the same, but be careful just in case */ - buf = malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); - bkg = malloc(nelmts * sizeof(struct dt)); - orig = malloc(nelmts * sizeof(struct st)); + buf = (unsigned char*)malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); + bkg = (unsigned char*)malloc(nelmts * sizeof(struct dt)); + orig = (unsigned char*)malloc(nelmts * sizeof(struct st)); for (i=0; i<(int)nelmts; i++) { s_ptr = ((struct st*)orig) + i; s_ptr->a = i*8+0; @@ -840,9 +840,9 @@ test_compound_3(void) TESTING("compound subset conversions"); /* Initialize */ - buf = malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); - bkg = malloc(nelmts * sizeof(struct dt)); - orig = malloc(nelmts * sizeof(struct st)); + buf = (unsigned char*)malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); + bkg = (unsigned char*)malloc(nelmts * sizeof(struct dt)); + orig = (unsigned char*)malloc(nelmts * sizeof(struct st)); for (i=0; i<(int)nelmts; i++) { s_ptr = ((struct st*)orig) + i; s_ptr->a = i*8+0; @@ -958,9 +958,9 @@ test_compound_4(void) TESTING("compound element shrinking & reordering"); /* Sizes should be the same, but be careful just in case */ - buf = malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); - bkg = malloc(nelmts * sizeof(struct dt)); - orig = malloc(nelmts * sizeof(struct st)); + buf = (unsigned char*)malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); + bkg = (unsigned char*)malloc(nelmts * sizeof(struct dt)); + orig = (unsigned char*)malloc(nelmts * sizeof(struct st)); for (i=0; i<(int)nelmts; i++) { s_ptr = ((struct st*)orig) + i; s_ptr->a = i*8+0; @@ -1187,9 +1187,9 @@ test_compound_6(void) TESTING("compound element growing"); /* Sizes should be the same, but be careful just in case */ - buf = malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); - bkg = malloc(nelmts * sizeof(struct dt)); - orig = malloc(nelmts * sizeof(struct st)); + buf = (unsigned char*)malloc(nelmts * MAX(sizeof(struct st), sizeof(struct dt))); + bkg = (unsigned char*)malloc(nelmts * sizeof(struct dt)); + orig = (unsigned char*)malloc(nelmts * sizeof(struct st)); for (i=0; i<(int)nelmts; i++) { s_ptr = ((struct st*)orig) + i; s_ptr->b = (i*8+1) & 0x7fff; @@ -2028,19 +2028,23 @@ test_compound_11(void) /* Verify converted buffer is correct */ for(u=0; u