[svn-r18820] Bug fix:

    Fixed a bug (1822) in the H5DSdetach_scale function when 0 bytes
    were allocated after the last reference to a dim. scale
    was removed from the list of references in a VL element of the
    DIMENSION_LIST attribute; modified the function to comply
    with the Spec: DIMENSION_LIST attribute is deleted now when no
    dimension scales left attached.

Platforms tested: jam with electric fence, amani amd linew

4 files changed, 244 insertions, 114 deletions

diff --git a/hl/src/H5DS.c b/hl/src/H5DS.c
index 245a2c5..936afd1 100644
--- a/hl/src/H5DS.c
+++ b/hl/src/H5DS.c
@@ -620,9 +620,11 @@ out:
 *
 * Date: December 20, 2004
 *
-* Comments:
+* Comments: 
 *
-* Modifications:
+* Modifications: Function didn't delete DIMENSION_LIST attribute, when 
+*                all dimension scales were detached from a dataset; added.
+*                                                           2010/05/13 EIP
 *
 *-------------------------------------------------------------------------
 */
@@ -642,14 +644,15 @@ herr_t H5DSdetach_scale(hid_t did,
     hid_t      aid = -1;     /* attribute ID */
     int        rank;         /* rank of dataset */
     ds_list_t  *dsbuf = NULL;  /* array of attribute data in the DS pointing to the dataset */
-    ds_list_t  *dsbufn = NULL; /* array of attribute data in the DS pointing to the dataset */
-    hsize_t    *dims = NULL;   /* dimension of the "REFERENCE_LIST" array */
+    hsize_t    dims[1];      /* dimension of the "REFERENCE_LIST" array */
     hobj_ref_t ref;          /* reference to the DS */
     hvl_t      *buf = NULL;  /* VL buffer to store in the attribute */
-    unsigned   i, j, jj;
-    H5O_info_t oi1, oi2, oi3, oi4;
+    int        i;
+    size_t     j;
+    hssize_t   ii;
+    H5O_info_t did_oi, dsid_oi, tmp_oi;
     int        found_dset = 0, found_ds = 0;
-    H5I_type_t it1, it2;
+    int        have_ds = 0;
     htri_t     is_scale;
 
     /*-------------------------------------------------------------------------
@@ -657,32 +660,28 @@ herr_t H5DSdetach_scale(hid_t did,
     *-------------------------------------------------------------------------
     */
 
-    if ((is_scale = H5DSis_scale(did)) < 0)
+    /* check for valid types of identifiers */
+
+    if(H5I_DATASET!=H5Iget_type(did) || H5I_DATASET!=H5Iget_type(dsid))
+        return FAIL;
+
+    if((is_scale = H5DSis_scale(did)) < 0)
         return FAIL;
 
     /* the dataset cannot be a DS dataset */
-    if ( is_scale == 1)
+    if( is_scale == 1)
         return FAIL;
 
     /* get info for the dataset in the parameter list */
-    if(H5Oget_info(did, &oi1) < 0)
+    if(H5Oget_info(did, &did_oi) < 0)
         return FAIL;
 
     /* get info for the scale in the parameter list */
-    if(H5Oget_info(dsid, &oi2) < 0)
+    if(H5Oget_info(dsid, &dsid_oi) < 0)
         return FAIL;
 
     /* same object, not valid */
-    if(oi1.fileno == oi2.fileno && oi1.addr == oi2.addr)
-        return FAIL;
-
-    /* get ID type */
-    if ((it1 = H5Iget_type(did)) < 0)
-        return FAIL;
-    if ((it2 = H5Iget_type(dsid)) < 0)
-        return FAIL;
-
-    if (H5I_DATASET!=it1 || H5I_DATASET!=it2)
+    if(did_oi.fileno == dsid_oi.fileno && did_oi.addr == dsid_oi.addr)
         return FAIL;
 
 
@@ -761,46 +760,62 @@ herr_t H5DSdetach_scale(hid_t did,
             if ((dsid_j = H5Rdereference(did,H5R_OBJECT,&ref)) < 0)
                 goto out;
 
-            /* get info for DS in the parameter list */
-            if(H5Oget_info(dsid, &oi1) < 0)
+            /* get info for this DS */
+            if(H5Oget_info(dsid_j, &tmp_oi) < 0)
                 goto out;
 
-            /* get info for this DS */
-            if(H5Oget_info(dsid_j, &oi2) < 0)
+            /* Close the dereferenced dataset */
+            if (H5Dclose(dsid_j) < 0)
                 goto out;
 
             /* same object, reset */
-            if(oi1.fileno == oi2.fileno && oi1.addr == oi2.addr)
+            if(dsid_oi.fileno == tmp_oi.fileno && dsid_oi.addr == tmp_oi.addr)
             {
-                size_t len;
-
-                for(jj=j; jj<buf[idx].len-1; jj++)
-                {
-                    ((hobj_ref_t *)buf[idx].p)[jj] = ((hobj_ref_t *)buf[idx].p)[jj+1];
+                /* If there are more than one reference in the VL element 
+                   and the reference we found is not the last one,
+                   copy the last one to replace the found one since the order
+                   of the references doesn't matter according to the spec; 
+                   reduce the size of the VL element by 1; 
+                   if the length of the element becomes 0, free the pointer 
+                   and reset to NULL */
+
+                size_t len = buf[idx].len;
+                if(j<len-1) 
+                    ((hobj_ref_t *)buf[idx].p)[j] = ((hobj_ref_t *)buf[idx].p)[len-1]; 
+                len = --buf[idx].len;
+                if(len == 0) {
+                    free(buf[idx].p); 
+                    buf[idx].p = NULL;
                 }
-
-                buf[idx].len--;
-                len = buf[idx].len;
-                buf[idx].p = realloc( buf[idx].p, len * sizeof(hobj_ref_t));
-
+                /* Since a reference to a dim. scale can be inserted only once, 
+                   we do not need to continue the search if it is found */
                 found_ds = 1;
-
-                /* close the dereferenced dataset and break */
-                if (H5Dclose(dsid_j) < 0)
-                    goto out;
                 break;
             }
-
-            /* close the dereferenced dataset */
-            if (H5Dclose(dsid_j) < 0)
-                goto out;
         } /* j */
     } /* if */
 
-    /* write the attribute */
-    if (H5Awrite(aid,tid,buf) < 0)
+    /* the scale must be present to continue */
+    if(found_ds == 0)
         goto out;
 
+    /* Write the attribute, but check first, if we have any scales left,
+       because if not, we should delete the attribute according to the spec */ 
+    for(i=0; i < rank; i++) {
+        if(buf[i].len > 0)  {
+            have_ds = 1;
+            break; 
+        }
+    }
+    if(have_ds) {        
+        if(H5Awrite(aid,tid,buf) < 0)
+            goto out; 
+    }
+    else {
+        if(H5Adelete(did,DIMENSION_LIST) < 0)
+            goto out;
+    }
+
     /* close */
     if (H5Dvlen_reclaim(tid,sid,H5P_DEFAULT,buf) < 0)
         goto out;
@@ -817,9 +832,6 @@ herr_t H5DSdetach_scale(hid_t did,
         buf = NULL;
     }
 
-    /* the scale must be present */
-    if(found_ds == 0)
-        goto out;
 
     /*-------------------------------------------------------------------------
     * the "REFERENCE_LIST" array exists, update
@@ -849,41 +861,36 @@ herr_t H5DSdetach_scale(hid_t did,
     if (H5Aread(aid,ntid,dsbuf) < 0)
         goto out;
 
-    for(i=0; i<nelmts; i++)
-    {
-        /* get the reference to the dataset */
-        ref = dsbuf[i].ref;
-
-        /* get the dataset id */
-        if ((did_i = H5Rdereference(did,H5R_OBJECT,&ref)) < 0)
-            goto out;
+    for(ii=0; ii<nelmts; ii++) {
+        /* First check if we have the same dimension index */
+        if((int)idx == dsbuf[ii].dim_idx) {
+            /* get the reference to the dataset */
+            ref = dsbuf[ii].ref;
 
-        /* get info for dataset in the parameter list */
-        if(H5Oget_info(did, &oi3) < 0)
-            goto out;
-
-        /* get info for this dataset */
-        if(H5Oget_info(did_i, &oi4) < 0)
-            goto out;
-
-        /* same object, reset. we want to detach only for this DIM */
-        if(oi3.fileno == oi4.fileno && oi3.addr == oi4.addr && (int)idx == dsbuf[i].dim_idx) {
-            for(jj=i; jj<nelmts-1; jj++)
-                dsbuf[jj] = dsbuf[jj+1];
-            nelmts--;
-            found_dset=1;
+            /* get the dataset id */
+            if ((did_i = H5Rdereference(did,H5R_OBJECT,&ref)) < 0)
+                goto out;
 
-            /* close the dereferenced dataset and break */
-            if (H5Dclose(did_i) < 0)
+            /* get info for this dataset */
+            if(H5Oget_info(did_i, &tmp_oi) < 0)
                 goto out;
-            break;
-        } /* if */
 
-        /* close the dereferenced dataset */
-        if (H5Dclose(did_i) < 0)
-            goto out;
+            /* close the dereferenced dataset */
+            if(H5Dclose(did_i) < 0)
+                goto out;
 
-    } /* i */
+            /* same object, reset. we want to detach only for this DIM */
+            if(did_oi.fileno == tmp_oi.fileno && did_oi.addr == tmp_oi.addr) {
+                /* if we found not the last one, copy the last one to replace 
+                   the one which is found */
+                if(i < nelmts-1)
+                    dsbuf[ii] = dsbuf[nelmts-1];
+                nelmts--;
+                found_dset=1;
+                break;
+            } /* if */
+        } /* if we have the same dimension index */
+    } /* ii */
 
     /* close space and attribute */
     if (H5Sclose(sid) < 0)
@@ -915,19 +922,8 @@ herr_t H5DSdetach_scale(hid_t did,
     if(nelmts)
     {
         /* create a new data space for the new references array */
-        dims = (hsize_t*)malloc((size_t)nelmts * sizeof (hsize_t));
-        if(dims == NULL)
-            goto out;
         dims[0] = nelmts;
 
-        dsbufn = (ds_list_t*) malloc((size_t)nelmts * sizeof(ds_list_t));
-        if(dsbufn == NULL)
-            goto out;
-
-        /* store the new information */
-        for(i = 0; i < nelmts; i++)
-            dsbufn[i] = dsbuf[i];
-
         if((sid = H5Screate_simple(1, dims, NULL)) < 0)
             goto out;
 
@@ -936,7 +932,7 @@ herr_t H5DSdetach_scale(hid_t did,
             goto out;
 
         /* write the new attribute with the new references */
-        if(H5Awrite(aid, tid, dsbufn) < 0)
+        if(H5Awrite(aid, tid, dsbuf) < 0)
             goto out;
 
         /* close space and attribute */
@@ -955,16 +951,6 @@ herr_t H5DSdetach_scale(hid_t did,
         free(dsbuf);
         dsbuf=NULL;
     }
-    if (dsbufn)
-    {
-        free(dsbufn);
-        dsbufn=NULL;
-    }
-    if (dims)
-    {
-        free(dims);
-        dims=NULL;
-    }
 
 
     return SUCCEED;
@@ -982,18 +968,14 @@ out:
             free(dsbuf);
             dsbuf=NULL;
         }
-        if (dsbufn)
-        {
-            free(dsbufn);
-            dsbufn=NULL;
-        }
-        if (dims)
-        {
-            free(dims);
-            dims=NULL;
-        }
         if (buf)
         {
+            /* Failure occured before H5Dvlen_reclaim was called;
+               free the pointers allocated when we read data in */
+            for(i=0; i<rank; i++) {
+                if(buf[i].p)
+                    free(buf[i].p);
+            }
             free(buf);
             buf=NULL;
         }
@@ -1218,7 +1200,7 @@ htri_t H5DSis_attached(hid_t did,
         if (dsbuf == NULL)
             goto out;
 
-        if (H5Aread(aid,ntid,dsbuf) < 0)
+        if (H5Aread(aid,ntid,dsbuf) < 0) 
             goto out;
 
         /*-------------------------------------------------------------------------
diff --git a/hl/test/Makefile.am b/hl/test/Makefile.am
index fc6e969..86e3318 100644
--- a/hl/test/Makefile.am
+++ b/hl/test/Makefile.am
@@ -46,6 +46,6 @@ endif
 # Temporary files.  These files are the ones created by running `make test'.
 CHECK_CLEANFILES+=combine_tables[1-2].h5 test_ds[1-9].h5 test_image[1-3].h5 \
         test_lite[1-2].h5 test_table.h5 test_packet_table.h5                \
-        test_packet_compress.h5
+        test_packet_compress.h5 test_detach.h5
 
 include $(top_srcdir)/config/conclude.am
diff --git a/hl/test/Makefile.in b/hl/test/Makefile.in
index f046110..aa6f13a 100644
--- a/hl/test/Makefile.in
+++ b/hl/test/Makefile.in
@@ -384,7 +384,8 @@ TRACE = perl $(top_srcdir)/bin/trace
 # Temporary files.  These files are the ones created by running `make test'.
 CHECK_CLEANFILES = *.chkexe *.chklog *.clog combine_tables[1-2].h5 \
 	test_ds[1-9].h5 test_image[1-3].h5 test_lite[1-2].h5 \
-	test_table.h5 test_packet_table.h5 test_packet_compress.h5
+	test_table.h5 test_packet_table.h5 test_packet_compress.h5 \
+	test_detach.h5
 
 # The tests depend on the hdf5, hdf5 test,  and hdf5_hl libraries
 LDADD = $(LIBH5_HL) $(LIBH5TEST) $(LIBHDF5)
diff --git a/hl/test/test_ds.c b/hl/test/test_ds.c
index 20baa2d..6b369f9 100644
--- a/hl/test/test_ds.c
+++ b/hl/test/test_ds.c
@@ -71,6 +71,7 @@ static int test_long_scalenames(const char *fileext);
 static int test_samelong_scalenames(const char *fileext);
 static int test_float_scalenames(const char *fileext);
 static int test_foreign_scaleattached(const char *fileforeign);
+static int test_detachscales(void);
 
 static int test_simple(void);
 static int test_errors(void);
@@ -135,6 +136,9 @@ static int read_data( const char* fname, int ndims, hsize_t *dims, float **buf )
 #define FILE5          "test_ds7.h5"
 #define FILE6          "test_ds8.h5"
 #define FILE7          "test_ds9.h5"
+ 
+#define DIMENSION_LIST "DIMENSION_LIST"
+#define REFERENCE_LIST "REFERENCE_LIST"
 
 /*-------------------------------------------------------------------------
  * the main program
@@ -169,6 +173,7 @@ int main(void)
     nerrors += test_samelong_scalenames("2") < 0  ? 1 : 0;
     nerrors += test_foreign_scaleattached(FOREIGN_FILE1) < 0  ? 1 : 0;
     nerrors += test_foreign_scaleattached(FOREIGN_FILE2) < 0  ? 1 : 0;
+    nerrors += test_detachscales() < 0  ? 1 : 0;
 
 /*  the following tests have not been rewritten to match those above */
     nerrors += test_simple() < 0  ?1:0;
@@ -1144,6 +1149,148 @@ herr_t test_cmp_scalename(hid_t fid, hid_t did, const char *name, const char *sc
     return ret_value;
 }
 
+static int test_detachscales(void)
+{
+    hid_t   fid = -1;
+    hid_t   did = -1;
+    hid_t   dsid = -1;
+    int     rank1 = 1;
+    int     rank3 = 3;
+    hsize_t dims[] = {1,2,3}; /*some bogus numbers, not important for the test*/
+    int     *buf = NULL;
+    char    dname[10];
+    int     i;
+
+    /* This tests creates two three dimensional datasets; then it creates
+       four integer datasets that are used as dim. scales; we will attach
+       and detach them to check that at the end there is no attributes
+       REFERENCE_LIST on a dimension scale and DIMENSION_LIST on a dataset */
+
+    TESTING2("test_detachscales");
+
+    if((fid = H5Fcreate("test_detach.h5", H5F_ACC_TRUNC, H5P_DEFAULT, H5P_DEFAULT)) < 0)
+        goto out;
+
+    /* make datasets; they are three dimensional*/
+    for (i=0; i < 2; i++) {
+        sprintf(dname,"D%d", i);
+        if(H5LTmake_dataset_int(fid, dname, rank3, dims, buf) < 0)
+            goto out;
+    } 
+    /* create datasets and make them dim. scales */
+
+    for (i=0; i < 4; i++) {
+        sprintf(dname, "DS%d", i);
+        if(H5LTmake_dataset_int(fid, dname, rank1, dims, buf) < 0)
+            goto out;
+    }
+    /* attach scales to the first dataset; first dimension will have 
+       two scales attached  */
+    if((did = H5Dopen2(fid, "D0", H5P_DEFAULT)) >= 0) {
+        for (i=0; i<4; i++) {
+           sprintf(dname, "DS%d", i);
+           if((dsid = H5Dopen2(fid, dname, H5P_DEFAULT)) < 0)
+               goto out;
+           if(H5DSattach_scale(did, dsid, (unsigned int) i%3) < 0)
+               goto out;
+           if(H5Dclose(dsid) < 0)
+               goto out;
+        }
+        if(H5Dclose(did) < 0)
+            goto out;
+    }
+    else
+        goto out;
+
+   /* attach scales to the second dataset */
+    if((did = H5Dopen2(fid, "D1", H5P_DEFAULT)) >= 0) {
+        for (i=0; i<3; i++) {
+           sprintf(dname, "DS%d", i);
+           if((dsid = H5Dopen2(fid, dname, H5P_DEFAULT)) < 0)
+               goto out;
+           if(H5DSattach_scale(did, dsid, (unsigned int) i) < 0)
+               goto out;
+           if(H5Dclose(dsid) < 0)
+               goto out;
+        }
+        if(H5Dclose(did) < 0)
+            goto out;
+    }
+    else
+        goto out;
+
+    /* detach DS0 from first dimension of D0 and D1; then check
+       that DS0 doesn't have attribute REFERENCE _LIST */
+
+    if((dsid = H5Dopen2(fid, "DS0", H5P_DEFAULT)) < 0)
+               goto out;
+
+    for (i=0; i<2; i++) {
+        sprintf(dname, "D%d", i);
+        if((did = H5Dopen2(fid, dname, H5P_DEFAULT)) < 0)
+            goto out;
+        if(H5DSdetach_scale(did, dsid, (unsigned int)0) < 0)
+            goto out;
+        if(H5Dclose(did) < 0)
+            goto out;
+    } 
+    /* Check that attribute "REFERENCE_LIST" doesn't exist anymore */
+    if(H5Aexists(dsid, REFERENCE_LIST)!= 0)
+        goto out;
+    if(H5Dclose(dsid) < 0)
+        goto out;
+    /* Check that DS3 is the only dim. scale attached to the first 
+       dimension of D0 */
+    if((did = H5Dopen2(fid, "D0", H5P_DEFAULT)) < 0)
+        goto out;
+    if((dsid = H5Dopen2(fid, "DS3", H5P_DEFAULT)) < 0)
+        goto out;
+    if(H5DSis_attached(did, dsid, (unsigned int) 0) <= 0)
+        goto out;
+    if(H5Dclose(did) < 0)
+        goto out;
+    if(H5Dclose(dsid) < 0)
+        goto out;
+
+    /* Detach the rest of the scales DS3, DS1, DS2 from D0 and make 
+       sure that attribute "DIMENSION_LIST" doesn't exist anymore */
+    if((did = H5Dopen2(fid, "D0", H5P_DEFAULT)) >= 0) {
+        for (i=1; i<4; i++) {
+           sprintf(dname, "DS%d", i);
+           if((dsid = H5Dopen2(fid, dname, H5P_DEFAULT)) < 0)
+               goto out;
+           if(H5DSdetach_scale(did, dsid, (unsigned int) i%3) < 0)
+               goto out;
+           if(H5Dclose(dsid) < 0)
+               goto out;
+        }
+        /* Check that attribute "DIMENSION_LIST" doesn't exist anymore */
+        if(H5Aexists(did, DIMENSION_LIST)!= 0) 
+            goto out;
+        if(H5Dclose(did) < 0)
+            goto out;
+    }
+    else
+        goto out;
+
+    
+    PASSED();
+
+    H5Fclose(fid);
+    return SUCCEED;
+
+out:
+    H5E_BEGIN_TRY  {
+        H5Dclose(did);
+        H5Dclose(dsid);
+        H5Fclose(fid);
+    } H5E_END_TRY;
+
+    H5_FAILED();
+
+    return FAIL;
+}
+
 static int test_char_attachscales(const char *fileext)
 {
     hid_t   fid = -1;