From c3be99abb19940db24fb3f5ff5fb1a5e78f832e6 Mon Sep 17 00:00:00 2001 From: Quincey Koziol Date: Wed, 3 Jan 2001 13:32:49 -0500 Subject: [svn-r3228] Purpose: Bug fixes Description: Fix two bugs: - Datasets with vlen datatype which were created but not written to were not being read back in correctly from the file. - If an existing space conversion path was found for a conversion, it was possible that the optimized read/write routines would be used inappropriately. Solution: Patched vlen datatype conversion code to correctly handle zero-length sequences. Added a check to the space conversion code to make certain that the optimized conversion routines are still appropriate when an existing path is found. Platforms tested: FreeBSD 4.2 (hawkwind) --- src/H5D.c | 47 ++++++++++++++++++------------ src/H5S.c | 53 +++++++++++++++++++++------------ src/H5Sall.c | 6 ++++ src/H5Tvlen.c | 94 +++++++++++++++++++++++++++++++++++------------------------ 4 files changed, 125 insertions(+), 75 deletions(-) diff --git a/src/H5D.c b/src/H5D.c index e2d8783..acf1742 100644 --- a/src/H5D.c +++ b/src/H5D.c @@ -1640,6 +1640,9 @@ H5D_read(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, "collective access for MPIO driver only"); #endif +#ifdef QAK +printf("%s: check 1.0, nelmts=%d\n",FUNC,(int)nelmts); +#endif /* QAK */ /* * Locate the type conversion function and data space conversion * functions, and set up the element numbering information. If a data @@ -1686,6 +1689,9 @@ H5D_read(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, } #endif /*H5_HAVE_PARALLEL*/ +#ifdef QAK +printf("%s: check 1.1, \n",FUNC); +#endif /* QAK */ /* * If there is no type conversion then try reading directly into the * application's buffer. This saves at least one mem-to-mem copy. @@ -1701,33 +1707,36 @@ H5D_read(H5D_t *dataset, const H5T_t *mem_type, const H5S_t *mem_space, mem_space, dxpl_id, buf/*out*/, &must_convert); if (status<0) { - /* Supports only no conversion, type or space, for now. */ - HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, + /* Supports only no conversion, type or space, for now. */ + HGOTO_ERROR(H5E_DATASET, H5E_READERROR, FAIL, "optimized read failed"); - } - if (must_convert) { - /* sconv->read cannot do a direct transfer; - * fall through and xfer the data in the more roundabout way */ - } else { - /* direct xfer accomplished successfully */ + } + if (must_convert) { + /* sconv->read cannot do a direct transfer; + * fall through and xfer the data in the more roundabout way */ + } else { + /* direct xfer accomplished successfully */ #ifdef H5S_DEBUG - H5_timer_end(&(sconv->stats[1].read_timer), &timer); - sconv->stats[1].read_nbytes += nelmts * - H5T_get_size(dataset->type); - sconv->stats[1].read_ncalls++; + H5_timer_end(&(sconv->stats[1].read_timer), &timer); + sconv->stats[1].read_nbytes += nelmts * + H5T_get_size(dataset->type); + sconv->stats[1].read_ncalls++; #endif - goto succeed; - } + goto succeed; + } #ifdef H5D_DEBUG - if (H5DEBUG(D)) { - fprintf (H5DEBUG(D), "H5D: data space conversion could not be " - "optimized for this case (using general method " - "instead)\n"); - } + if (H5DEBUG(D)) { + fprintf (H5DEBUG(D), "H5D: data space conversion could not be " + "optimized for this case (using general method " + "instead)\n"); + } #endif H5E_clear (); } +#ifdef QAK +printf("%s: check 1.2, \n",FUNC); +#endif /* QAK */ #ifdef H5_HAVE_PARALLEL /* The following may not handle a collective call correctly * since it does not ensure all processes can handle the read diff --git a/src/H5S.c b/src/H5S.c index 5a21387..3921fbc 100644 --- a/src/H5S.c +++ b/src/H5S.c @@ -1523,10 +1523,26 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space) * If so then return a pointer to that entry. */ for (i=0; if->type==file_space->select.type && - H5S_conv_g[i]->m->type==mem_space->select.type) { - HRETURN(H5S_conv_g[i]); - } + if (H5S_conv_g[i]->f->type==file_space->select.type && + H5S_conv_g[i]->m->type==mem_space->select.type) { + /* + * Initialize direct read/write functions + */ + c1=H5S_select_contiguous(file_space); + c2=H5S_select_contiguous(mem_space); + if(c1==FAIL || c2==FAIL) + HRETURN_ERROR(H5E_DATASPACE, H5E_BADRANGE, NULL, + "invalid check for contiguous dataspace "); + + if (c1==TRUE && c2==TRUE) { + H5S_conv_g[i]->read = H5S_all_read; + H5S_conv_g[i]->write = H5S_all_write; + } + else + H5S_conv_g[i]->read = H5S_conv_g[i]->write = NULL; + + HRETURN(H5S_conv_g[i]); + } } /* @@ -1534,8 +1550,8 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space) * path? */ if (NULL==H5S_fconv_g[file_space->select.type] || - NULL==H5S_mconv_g[mem_space->select.type]) { - HRETURN_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, NULL, + NULL==H5S_mconv_g[mem_space->select.type]) { + HRETURN_ERROR(H5E_DATASPACE, H5E_UNSUPPORTED, NULL, "unable to convert between data space selections"); } @@ -1543,7 +1559,7 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space) * Create a new path. */ if (NULL==(path = H5MM_calloc(sizeof(*path)))) { - HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, + HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, "memory allocation failed for data space conversion " "path"); } @@ -1560,23 +1576,24 @@ H5S_find (const H5S_t *mem_space, const H5S_t *file_space) "invalid check for contiguous dataspace "); if (c1==TRUE && c2==TRUE) { - path->read = H5S_all_read; - path->write = H5S_all_write; + path->read = H5S_all_read; + path->write = H5S_all_write; } /* * Add the new path to the table. */ if (H5S_nconv_g>=H5S_aconv_g) { - size_t n = MAX(10, 2*H5S_aconv_g); - H5S_conv_t **p = H5MM_realloc(H5S_conv_g, n*sizeof(H5S_conv_g[0])); - if (NULL==p) { - HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, - "memory allocation failed for data space conversion " - "path table"); - } - H5S_aconv_g = n; - H5S_conv_g = p; + size_t n = MAX(10, 2*H5S_aconv_g); + H5S_conv_t **p = H5MM_realloc(H5S_conv_g, n*sizeof(H5S_conv_g[0])); + + if (NULL==p) { + HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, NULL, + "memory allocation failed for data space conversion " + "path table"); + } + H5S_aconv_g = n; + H5S_conv_g = p; } H5S_conv_g[H5S_nconv_g++] = path; diff --git a/src/H5Sall.c b/src/H5Sall.c index 4240271..f56e480 100644 --- a/src/H5Sall.c +++ b/src/H5Sall.c @@ -572,6 +572,9 @@ H5S_all_read(H5F_t *f, const H5O_layout_t *layout, const H5O_pline_t *pline, FUNC_ENTER(H5S_all_read, FAIL); *must_convert = TRUE; +#ifdef QAK +printf("%s: check 1.0\n",FUNC); +#endif /* QAK */ /* Check whether we can handle this */ if (H5S_SIMPLE!=mem_space->extent.type) goto fall_through; @@ -674,6 +677,9 @@ H5S_all_read(H5F_t *f, const H5O_layout_t *layout, const H5O_pline_t *pline, file_offset[i] = 0; mem_offset[i] = 0; +#ifdef QAK +printf("%s: check 2.0\n",FUNC); +#endif /* QAK */ /* Read data from the file */ if (H5F_arr_read(f, dxpl_id, layout, pline, NULL, efl, size, size, mem_offset, file_offset, buf/*out*/)<0) { diff --git a/src/H5Tvlen.c b/src/H5Tvlen.c index ea2f7c0..3957f79 100644 --- a/src/H5Tvlen.c +++ b/src/H5Tvlen.c @@ -217,18 +217,26 @@ herr_t H5T_vlen_seq_mem_write(const H5D_xfer_t *xfer_parms, H5F_t UNUSED *f, voi assert(vl); assert(buf); - /* Use the user's memory allocation routine is one is defined */ - if(xfer_parms->vlen_alloc!=NULL) { - if(NULL==(vl->p=(xfer_parms->vlen_alloc)(seq_len*base_size,xfer_parms->alloc_info))) - HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for VL data"); - } /* end if */ - else { /* Default to system malloc */ - if(NULL==(vl->p=H5MM_malloc(seq_len*base_size))) - HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for VL data"); - } /* end else */ - vl->len=seq_len; + if(seq_len!=0) { + /* Use the user's memory allocation routine is one is defined */ + if(xfer_parms->vlen_alloc!=NULL) { + if(NULL==(vl->p=(xfer_parms->vlen_alloc)(seq_len*base_size,xfer_parms->alloc_info))) + HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for VL data"); + } /* end if */ + else { /* Default to system malloc */ + if(NULL==(vl->p=H5MM_malloc(seq_len*base_size))) + HRETURN_ERROR(H5E_RESOURCE, H5E_NOSPACE, FAIL, "memory allocation failed for VL data"); + } /* end else */ + + /* Copy the data into the newly allocated buffer */ + HDmemcpy(vl->p,buf,len); + + } /* end if */ + else + vl->p=NULL; - HDmemcpy(vl->p,buf,len); + /* Set the sequence length */ + vl->len=seq_len; FUNC_LEAVE (SUCCEED); } /* end H5T_vlen_seq_mem_write() */ @@ -395,13 +403,16 @@ herr_t H5T_vlen_disk_read(H5F_t *f, void *vl_addr, void *buf, size_t UNUSED len) /* Get the length of the sequence */ UINT32DECODE(vl, seq_len); /* Not used */ - /* Get the heap information */ - H5F_addr_decode(f,(const uint8_t **)&vl,&(hobjid.addr)); - INT32DECODE(vl,hobjid.idx); - - /* Read the VL information from disk */ - if(H5HG_read(f,&hobjid,buf)==NULL) - HRETURN_ERROR(H5E_DATATYPE, H5E_READERROR, FAIL, "Unable to read VL information"); + /* Check if this sequence actually has any data */ + if(seq_len!=0) { + /* Get the heap information */ + H5F_addr_decode(f,(const uint8_t **)&vl,&(hobjid.addr)); + INT32DECODE(vl,hobjid.idx); + + /* Read the VL information from disk */ + if(H5HG_read(f,&hobjid,buf)==NULL) + HRETURN_ERROR(H5E_DATATYPE, H5E_READERROR, FAIL, "Unable to read VL information"); + } /* end if */ FUNC_LEAVE (SUCCEED); } /* end H5T_vlen_disk_read() */ @@ -437,9 +448,14 @@ herr_t H5T_vlen_disk_write(const H5D_xfer_t UNUSED *xfer_parms, H5F_t *f, void * /* Set the length of the sequence */ UINT32ENCODE(vl, seq_len); - /* Write the VL information to disk (allocates space also) */ - if(H5HG_insert(f,len,buf,&hobjid)<0) - HRETURN_ERROR(H5E_DATATYPE, H5E_WRITEERROR, FAIL, "Unable to write VL information"); + /* Check if this sequence actually has any data */ + if(seq_len!=0) { + /* Write the VL information to disk (allocates space also) */ + if(H5HG_insert(f,len,buf,&hobjid)<0) + HRETURN_ERROR(H5E_DATATYPE, H5E_WRITEERROR, FAIL, "Unable to write VL information"); + } /* end if */ + else + HDmemset(&hobjid,0,sizeof(H5HG_t)); /* Get the heap information */ H5F_addr_encode(f,&vl,hobjid.addr); @@ -518,25 +534,27 @@ H5T_vlen_reclaim_recurse(void *elem, H5T_t *dt, H5MM_free_t free_func, void *fre if(dt->u.vlen.type==H5T_VLEN_SEQUENCE) { hvl_t *vl=(hvl_t *)elem; /* Temp. ptr to the vl info */ - /* Recurse if it's VL or compound */ - if(dt->parent->type==H5T_COMPOUND || dt->parent->type==H5T_VLEN || dt->parent->type==H5T_ARRAY) { - void *off; /* offset of field */ + /* Check if there is anything actually in this sequence */ + if(vl->len!=0) { + /* Recurse if it's VL or compound */ + if(dt->parent->type==H5T_COMPOUND || dt->parent->type==H5T_VLEN || dt->parent->type==H5T_ARRAY) { + void *off; /* offset of field */ + + /* Calculate the offset of each array element and recurse on it */ + while(vl->len>0) { + off=((uint8_t *)vl->p)+(vl->len-1)*dt->parent->size; + if(H5T_vlen_reclaim_recurse(off,dt->parent,free_func,free_info)<0) + HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "Unable to free VL element"); + vl->len--; + } /* end while */ + } /* end if */ - /* Calculate the offset of each array element and recurse on it */ - while(vl->len>0) { - off=((uint8_t *)vl->p)+(vl->len-1)*dt->parent->size; - if(H5T_vlen_reclaim_recurse(off,dt->parent,free_func,free_info)<0) - HGOTO_ERROR(H5E_DATATYPE, H5E_CANTFREE, FAIL, "Unable to free VL element"); - vl->len--; - } /* end while */ + /* Free the VL sequence */ + if(free_func!=NULL) + (*free_func)(vl->p,free_info); + else + H5MM_xfree(vl->p); } /* end if */ - - /* Free the VL sequence */ - if(free_func!=NULL) - (*free_func)(vl->p,free_info); - else { - H5MM_xfree(vl->p); - } } else if(dt->u.vlen.type==H5T_VLEN_STRING) { /* Free the VL string */ if(free_func!=NULL) -- cgit v0.12