Repository: Unidata/netcdf-c (c)

Issue:
HDF5 error when unlimited dimension and a variable share the same name
Reported by a python user (https://github.com/Unidata/netcdf4-python/issues/975).  The error occurs when a variable has the same name as an unlimited dimension, but does not have an unlimited dimension.  Both C and python programs to trigger the error included in the python issue.


Pull request description:
Fix for subtle error involving var and unlimited dim of the same name, but unrelated, in netCDF-4
Fixes #1496 

When looking up the coordinate var associated with a dimension, we were looking it up by name. Well, if there is a var of the same name, which is not a coordinate var, hilarious confusion ensues.

Now, instead of checking by name, we use the dim->coord_var pointer, which already correctly points to a coord var, or is NULL if there is no coord var.

I have added code derived from @jswhit to test.

Patch:
diff --git a/libhdf5/hdf5var.c b/libhdf5/hdf5var.c
index bf0b7e8a1..a48b03564 100644
--- a/libhdf5/hdf5var.c
+++ b/libhdf5/hdf5var.c
@@ -15,9 +15,24 @@
 #endif
 #include <math.h> /* For pow() used below. */
 
+/** @internal Default size for unlimited dim chunksize. */
+#define DEFAULT_1D_UNLIM_SIZE (4096)
+
+/** @internal Temp name used when renaming vars to preserve varid
+ * order. */
+#define NC_TEMP_NAME "_netcdf4_temporary_variable_name_for_rename"
+
 #ifdef LOGGING
+/**
+ * Report the chunksizes selected for a variable.
+ *
+ * @param title A text title for the report.
+ * @param var Pointer to the var of interest.
+ *
+ * @author Dennis Heimbigner
+ */
 static void
-reportchunking(const char* title, NC_VAR_INFO_T* var)
+reportchunking(const char *title, NC_VAR_INFO_T *var)
 {
     int i;
     char buf[8192];
@@ -33,17 +48,10 @@ reportchunking(const char* title, NC_VAR_INFO_T* var)
         snprintf(digits,sizeof(digits),"%ld",(unsigned long)var->chunksizes[i]);
 	strlcat(buf,digits,sizeof(buf));
     }
-    LOG((1,"%s",buf));
-}    
+    LOG((3,"%s",buf));
+}
 #endif
 
-/** @internal Default size for unlimited dim chunksize. */
-#define DEFAULT_1D_UNLIM_SIZE (4096)
-
-/** @internal Temp name used when renaming vars to preserve varid
- * order. */
-#define NC_TEMP_NAME "_netcdf4_temporary_variable_name_for_rename"
-
 /**
  * @internal If the HDF5 dataset for this variable is open, then close
  * it and reopen it, with the perhaps new settings for chunk caching.
@@ -247,7 +255,7 @@ nc4_find_default_chunksizes2(NC_GRP_INFO_T *grp, NC_VAR_INFO_T *var)
     }
 
 #ifdef LOGGING
-reportchunking("find_default: ",var);
+    reportchunking("find_default: ",var);
 #endif
     return NC_NOERR;
 }
@@ -753,9 +761,10 @@ nc_def_var_extra(int ncid, int varid, int *shuffle, int *deflate,
     }
 
 #ifdef LOGGING
-{int dfalt=(chunksizes == NULL);
-reportchunking(dfalt?"extra: default: ":"extra: user: ",var);
-}
+    {
+        int dfalt = (chunksizes == NULL);
+        reportchunking(dfalt ? "extra: default: " : "extra: user: ", var);
+    }
 #endif
 
     /* Are we setting a fill modes? */
@@ -1653,7 +1662,7 @@ NC4_put_vars(int ncid, int varid, const size_t *startp, const size_t *countp,
 #endif
                 if (!zero_count && endindex >= fdims[d2])
                 {
-                    xtend_size[d2] = (long long unsigned)(endindex+1);
+                    xtend_size[d2] = (long long unsigned)(endindex + 1);
                     need_to_extend++;
                 }
                 else
@@ -1661,7 +1670,7 @@ NC4_put_vars(int ncid, int varid, const size_t *startp, const size_t *countp,
 
                 if (!zero_count && endindex >= dim->len)
                 {
-                    dim->len = endindex+1;
+                    dim->len = endindex + 1;
                     dim->extended = NC_TRUE;
                 }
             }
diff --git a/libhdf5/nc4hdf.c b/libhdf5/nc4hdf.c
index ae3ef6ff1..fca9533d4 100644
--- a/libhdf5/nc4hdf.c
+++ b/libhdf5/nc4hdf.c
@@ -1863,10 +1863,10 @@ write_dim(NC_DIM_INFO_T *dim, NC_GRP_INFO_T *grp, nc_bool_t write_dimid)
         NC_VAR_INFO_T *v1 = NULL;
 
         assert(dim->unlimited);
-        /* If this is a dimension without a variable, then update
-         * the secret length information at the end of the NAME
-         * attribute. */
-        v1 = (NC_VAR_INFO_T *)ncindexlookup(grp->vars, dim->hdr.name);
+
+        /* If this is a dimension with an associated coordinate var,
+         * then update the length of that coord var. */
+        v1 = dim->coord_var;
         if (v1)
         {
             NC_HDF5_VAR_INFO_T *hdf5_v1;


Test patch:
diff --git a/nc_test4/tst_dims3.c b/nc_test4/tst_dims3.c
index 92978f608..c98ec5e20 100644
--- a/nc_test4/tst_dims3.c
+++ b/nc_test4/tst_dims3.c
@@ -2,9 +2,10 @@
    Corporation for Atmospheric Research/Unidata See COPYRIGHT file for
    conditions of use. See www.unidata.ucar.edu for more info.
 
-   Test netcdf-4 dimensions inheritance.
+   Test netcdf-4 dimensions inheritance, and dims with and without
+   coordinate variables.
 
-   $Id: tst_dims3.c,v 1.7 2010/05/25 13:53:04 ed Exp $
+   Ed Hartnett
 */
 
 #include <config.h>
@@ -198,5 +199,41 @@ main(int argc, char **argv)
 	ERR_RET;
    }
    SUMMARIZE_ERR;
+   printf("*** testing var and unlim dim with same name, but not related...");
+   {
+       /* This test code based on test code from Jeff Whitaker. See
+        * https://github.com/Unidata/netcdf4-python/issues/975 and
+        * https://github.com/Unidata/netcdf-c/issues/1496. */
+       int ncid, timesubset_id, time_id, timevar_id, dummyvar_id;
+       size_t start[1] = {0};
+       size_t count[1] = {1};
+       double data[1] = {TEST_VAL_42};
+       size_t len;
+       double data_in;
+
+       if (nc_create(FILE_NAME, NC_CLOBBER | NC_NETCDF4, &ncid)) ERR;
+       if (nc_def_dim(ncid, "time", NC_UNLIMITED, &time_id)) ERR;
+       if (nc_def_dim(ncid, "time_subset", 50, &timesubset_id)) ERR;
+
+       /* Define vars. */
+       if (nc_def_var(ncid, "time", NC_DOUBLE, 1, &timesubset_id, &timevar_id)) ERR;
+       if (nc_def_var(ncid, "dummy", NC_DOUBLE, 1, &time_id, &dummyvar_id)) ERR;
+       if (nc_enddef(ncid)) ERR;
+
+       /* Write some data. */
+       if (nc_put_vara(ncid, dummyvar_id, start, count, data)) ERR;
+
+       /* Close the file. */
+       if (nc_close(ncid)) ERR;
+
+       /* Reopen file and check. */
+       if (nc_open(FILE_NAME, NC_WRITE, &ncid)) ERR;
+       if (nc_inq_dim(ncid, 0, NULL, &len)) ERR;
+       if (len != 1) ERR;
+       if (nc_get_vara_double(ncid, 1, start, count, &data_in)) ERR;
+       if (data_in != TEST_VAL_42) ERR;
+       if (nc_close(ncid)) ERR;
+   }
+   SUMMARIZE_ERR;
    FINAL_RESULTS;
 }