Fix some issues, add comments, and prepare for lazy(-ish) var reads

include/hdf5internal.h

@@ -105,7 +105,6 @@ int hdf5_set_log_level();
 int rec_detach_scales(NC_GRP_INFO_T *grp, int dimid, hid_t dimscaleid);
 int rec_reattach_scales(NC_GRP_INFO_T *grp, int dimid, hid_t dimscaleid);
 int delete_existing_dimscale_dataset(NC_GRP_INFO_T *grp, int dimid, NC_DIM_INFO_T *dim);
-int nc4_rec_match_dimscales(NC_GRP_INFO_T *grp);
 
 /* Write metadata. */
 int nc4_rec_write_metadata(NC_GRP_INFO_T *grp, nc_bool_t bad_coord_order);
@@ -137,7 +136,6 @@ extern hid_t NC4_image_init(NC_FILE_INFO_T* h5);
 extern void NC4_image_finalize(void*);
 
 /* These functions are internal to the libhdf5 directory. */
-int nc4_detect_preserve_dimids(NC_GRP_INFO_T *grp, nc_bool_t *bad_coord_orderp);
 int nc4_get_fill_value(NC_FILE_INFO_T *h5, NC_VAR_INFO_T *var, void **fillp);
 
 

libhdf5/hdf5file.c

@@ -75,6 +75,84 @@ NC_findreserved(const char* name)
    return NULL;
 }
 
+/**
+ * @internal Recursively determine if there is a mismatch between
+ * order of coordinate creation and associated dimensions in this
+ * group or any subgroups, to find out if we have to handle that
+ * situation.  Also check if there are any multidimensional coordinate
+ * variables defined, which require the same treatment to fix a
+ * potential bug when such variables occur in subgroups.
+ *
+ * @param grp Pointer to group info struct.
+ * @param bad_coord_orderp Pointer that gets 1 if there is a bad
+ * coordinate order.
+ *
+ * @returns NC_NOERR No error.
+ * @returns NC_EHDFERR HDF5 returned an error.
+ * @author Ed Hartnett
+ */
+static int
+detect_preserve_dimids(NC_GRP_INFO_T *grp, nc_bool_t *bad_coord_orderp)
+{
+   NC_VAR_INFO_T *var;
+   NC_GRP_INFO_T *child_grp;
+   int last_dimid = -1;
+   int retval;
+   int i;
+
+   /* Iterate over variables in this group */
+   for (i=0; i < ncindexsize(grp->vars); i++)
+   {
+      var = (NC_VAR_INFO_T*)ncindexith(grp->vars,i);
+      if (var == NULL) continue;
+      /* Only matters for dimension scale variables, with non-scalar dimensionality */
+      if (var->dimscale && var->ndims)
+      {
+         /* If the user writes coord vars in a different order then he
+          * defined their dimensions, then, when the file is reopened, the
+          * order of the dimids will change to match the order of the coord
+          * vars. Detect if this is about to happen. */
+         if (var->dimids[0] < last_dimid)
+         {
+            LOG((5, "%s: %s is out of order coord var", __func__, var->hdr.name));
+            *bad_coord_orderp = NC_TRUE;
+            return NC_NOERR;
+         }
+         last_dimid = var->dimids[0];
+
+         /* If there are multidimensional coordinate variables defined, then
+          * it's also necessary to preserve dimension IDs when the file is
+          * reopened ... */
+         if (var->ndims > 1)
+         {
+            LOG((5, "%s: %s is multidimensional coord var", __func__, var->hdr.name));
+            *bad_coord_orderp = NC_TRUE;
+            return NC_NOERR;
+         }
+
+         /* Did the user define a dimension, end define mode, reenter define
+          * mode, and then define a coordinate variable for that dimension?
+          * If so, dimensions will be out of order. */
+         if (var->is_new_var || var->became_coord_var)
+         {
+            LOG((5, "%s: coord var defined after enddef/redef", __func__));
+            *bad_coord_orderp = NC_TRUE;
+            return NC_NOERR;
+         }
+      }
+   }
+
+   /* If there are any child groups, check them also for this condition. */
+   for (i = 0; i < ncindexsize(grp->children); i++)
+   {
+      if (!(child_grp = (NC_GRP_INFO_T *)ncindexith(grp->children, i)))
+         continue;
+      if ((retval = detect_preserve_dimids(child_grp, bad_coord_orderp)))
+         return retval;
+   }
+   return NC_NOERR;
+}
+
 /**
  * @internal This function will write all changed metadata and flush
  * HDF5 file to disk.
@@ -127,7 +205,7 @@ sync_netcdf4_file(NC_FILE_INFO_T *h5)
       /* Check to see if the coordinate order is messed up. If
        * detected, propagate to all groups to consistently store
        * dimids. */
-      if ((retval = nc4_detect_preserve_dimids(h5->root_grp, &bad_coord_order)))
+      if ((retval = detect_preserve_dimids(h5->root_grp, &bad_coord_order)))
          return retval;
 
       /* Write all the metadata. */

libhdf5/hdf5internal.c

@@ -106,7 +106,8 @@ nc4_hdf5_finalize(void)
  * @author Ed Hartnett
  */
 static int
-find_var_dim_max_length(NC_GRP_INFO_T *grp, int varid, int dimid, size_t *maxlen)
+find_var_dim_max_length(NC_GRP_INFO_T *grp, int varid, int dimid,
+                        size_t *maxlen)
 {
    hid_t datasetid = 0, spaceid = 0;
    NC_VAR_INFO_T *var;
@@ -230,30 +231,27 @@ nc4_rec_find_hdf_type(NC_FILE_INFO_T *h5, hid_t target_hdf_typeid)
 int
 nc4_find_dim_len(NC_GRP_INFO_T *grp, int dimid, size_t **len)
 {
-   NC_GRP_INFO_T *g;
    NC_VAR_INFO_T *var;
    int retval;
    int i;
 
    assert(grp && len);
-   LOG((3, "nc4_find_dim_len: grp->name %s dimid %d", grp->hdr.name, dimid));
+   LOG((3, "%s: grp->name %s dimid %d", __func__, grp->hdr.name, dimid));
 
    /* If there are any groups, call this function recursively on
     * them. */
-   for(i=0;i<ncindexsize(grp->children);i++) {
-      g = (NC_GRP_INFO_T*)ncindexith(grp->children,i);
-      if(g == NULL) continue;
-      if ((retval = nc4_find_dim_len(g, dimid, len)))
+   for (i = 0; i < ncindexsize(grp->children); i++)
+      if ((retval = nc4_find_dim_len((NC_GRP_INFO_T*)ncindexith(grp->children, i),
+                                     dimid, len)))
          return retval;
-   }
 
    /* For all variables in this group, find the ones that use this
     * dimension, and remember the max length. */
-   for (i=0; i < ncindexsize(grp->vars); i++)
+   for (i = 0; i < ncindexsize(grp->vars); i++)
    {
       size_t mylen;
-      var = (NC_VAR_INFO_T*)ncindexith(grp->vars,i);
-      if (var == NULL) continue;
+      var = (NC_VAR_INFO_T *)ncindexith(grp->vars, i);
+      assert(var);
 
       /* Find max length of dim in this variable... */
       if ((retval = find_var_dim_max_length(grp, var->hdr.id, dimid, &mylen)))
@@ -283,7 +281,8 @@ nc4_find_dim_len(NC_GRP_INFO_T *grp, int dimid, size_t **len)
  * @author Quincey Koziol, Ed Hartnett
  */
 int
-nc4_break_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *coord_var, NC_DIM_INFO_T *dim)
+nc4_break_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *coord_var,
+                    NC_DIM_INFO_T *dim)
 {
    int retval;
 
@@ -314,10 +313,6 @@ nc4_break_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *coord_var, NC_DIM_INFO_T
          return NC_ENOMEM;
    }
 
-   /* Remove the atts that go with being a coordinate var. */
-   /* if ((retval = remove_coord_atts(coord_var->hdf_datasetid))) */
-   /*    return retval; */
-
    /* Detach dimension from variable */
    coord_var->dimscale = NC_FALSE;
    dim->coord_var = NULL;
@@ -351,7 +346,8 @@ nc4_break_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *coord_var, NC_DIM_INFO_T
  * @author Ed Hartnett
  */
 int
-delete_existing_dimscale_dataset(NC_GRP_INFO_T *grp, int dimid, NC_DIM_INFO_T *dim)
+delete_existing_dimscale_dataset(NC_GRP_INFO_T *grp, int dimid,
+                                 NC_DIM_INFO_T *dim)
 {
    NC_HDF5_DIM_INFO_T *hdf5_dim;
    NC_HDF5_GRP_INFO_T *hdf5_grp;
@@ -512,39 +508,20 @@ nc4_reform_coord_var(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var, NC_DIM_INFO_T *dim)
 }
 
 /**
- * @internal Recursively free HDF5 objects for a group (and everything
- * it contains).
+ * @internal Close HDF5 resources for global atts in a group.
  *
  * @param grp Pointer to group info struct.
  *
  * @return ::NC_NOERR No error.
+ * @return ::NC_EHDFERR HDF5 error.
  * @author Ed Hartnett
  */
-int
-nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
+static int
+close_gatts(NC_GRP_INFO_T *grp)
 {
-   NC_VAR_INFO_T *var;
-   NC_HDF5_VAR_INFO_T *hdf5_var;
-   NC_DIM_INFO_T *dim;
    NC_ATT_INFO_T *att;
-   NC_HDF5_GRP_INFO_T *hdf5_grp;
    int a;
-   int i;
-   int retval;
-
-   assert(grp && grp->format_grp_info);
-   LOG((3, "%s: grp->name %s", __func__, grp->hdr.name));
 
-   hdf5_grp = (NC_HDF5_GRP_INFO_T *)grp->format_grp_info;
-
-   /* Recursively call this function for each child, if any, stopping
-    * if there is an error. */
-   for (i = 0; i < ncindexsize(grp->children); i++)
-      if ((retval = nc4_rec_grp_HDF5_del((NC_GRP_INFO_T *)ncindexith(grp->children,
-                                                                     i))))
-         return retval;
-
-   /* Close HDF5 resources associated with global attributes. */
    for (a = 0; a < ncindexsize(grp->att); a++)
    {
       NC_HDF5_ATT_INFO_T *hdf5_att;
@@ -558,8 +535,26 @@ nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
           H5Tclose(hdf5_att->native_hdf_typeid) < 0)
          return NC_EHDFERR;
    }
+   return NC_NOERR;
+}
+
+/**
+ * @internal Close HDF5 resources for vars in a group.
+ *
+ * @param grp Pointer to group info struct.
+ *
+ * @return ::NC_NOERR No error.
+ * @return ::NC_EHDFERR HDF5 error.
+ * @author Ed Hartnett
+ */
+static int
+close_vars(NC_GRP_INFO_T *grp)
+{
+   NC_VAR_INFO_T *var;
+   NC_HDF5_VAR_INFO_T *hdf5_var;
+   NC_ATT_INFO_T *att;
+   int a, i;
 
-   /* Close HDF5 resources associated with vars. */
    for (i = 0; i < ncindexsize(grp->vars); i++)
    {
       var = (NC_VAR_INFO_T *)ncindexith(grp->vars, i);
@@ -603,7 +598,24 @@ nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
       }
    }
 
-   /* Close HDF5 resources associated with dims. */
+   return NC_NOERR;
+}
+
+/**
+ * @internal Close HDF5 resources for dims in a group.
+ *
+ * @param grp Pointer to group info struct.
+ *
+ * @return ::NC_NOERR No error.
+ * @return ::NC_EHDFERR HDF5 error.
+ * @author Ed Hartnett
+ */
+static int
+close_dims(NC_GRP_INFO_T *grp)
+{
+   NC_DIM_INFO_T *dim;
+   int i;
+
    for (i = 0; i < ncindexsize(grp->dim); i++)
    {
       NC_HDF5_DIM_INFO_T *hdf5_dim;
@@ -619,9 +631,25 @@ nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
          return NC_EHDFERR;
    }
 
-   /* Close HDF5 resources associated with types. Set values to 0
-    * after closing types. Because of type reference counters, these
-    * closes can be called multiple times. */
+   return NC_NOERR;
+}
+
+/**
+ * @internal Close HDF5 resources for types in a group.  Set values to
+ * 0 after closing types. Because of type reference counters, these
+ * closes can be called multiple times.
+ *
+ * @param grp Pointer to group info struct.
+ *
+ * @return ::NC_NOERR No error.
+ * @return ::NC_EHDFERR HDF5 error.
+ * @author Ed Hartnett, Dennis Heimbigner
+ */
+static int
+close_types(NC_GRP_INFO_T *grp)
+{
+   int i;
+
    for (i = 0; i < ncindexsize(grp->type); i++)
    {
       NC_TYPE_INFO_T *type;
@@ -643,6 +671,54 @@ nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
       hdf5_type->native_hdf_typeid = 0;
    }
 
+   return NC_NOERR;
+}
+
+/**
+ * @internal Recursively free HDF5 objects for a group (and everything
+ * it contains).
+ *
+ * @param grp Pointer to group info struct.
+ *
+ * @return ::NC_NOERR No error.
+ * @return ::NC_EHDFERR HDF5 error.
+ * @author Ed Hartnett
+ */
+int
+nc4_rec_grp_HDF5_del(NC_GRP_INFO_T *grp)
+{
+   NC_HDF5_GRP_INFO_T *hdf5_grp;
+   int i;
+   int retval;
+
+   assert(grp && grp->format_grp_info);
+   LOG((3, "%s: grp->name %s", __func__, grp->hdr.name));
+
+   hdf5_grp = (NC_HDF5_GRP_INFO_T *)grp->format_grp_info;
+
+   /* Recursively call this function for each child, if any, stopping
+    * if there is an error. */
+   for (i = 0; i < ncindexsize(grp->children); i++)
+      if ((retval = nc4_rec_grp_HDF5_del((NC_GRP_INFO_T *)ncindexith(grp->children,
+                                                                     i))))
+         return retval;
+
+   /* Close HDF5 resources associated with global attributes. */
+   if ((retval = close_gatts(grp)))
+      return retval;
+
+   /* Close HDF5 resources associated with vars. */
+   if ((retval = close_vars(grp)))
+      return retval;
+
+   /* Close HDF5 resources associated with dims. */
+   if ((retval = close_dims(grp)))
+      return retval;
+
+   /* Close HDF5 resources associated with types. */
+   if ((retval = close_types(grp)))
+      return retval;
+
    /* Close the HDF5 group. */
    LOG((4, "%s: closing group %s", __func__, grp->hdr.name));
    if (hdf5_grp->hdf_grpid && H5Gclose(hdf5_grp->hdf_grpid) < 0)