Repository: Unidata/netcdf-c (c)

Issue:
User-defined formats must take priority in NC_infermodel()
I'm glad I caught this before the next release, because it breaks user-defined formats in some cases.

The determination that this is a user-defined format must be done before any other cmode/omode flags. That's because the user-defined format may use some of the same flags as netCDF, but needs to handle them in the UDF code.

For example:

`nc_create("file.nc", NC_PIO|NC_NETCDF4, &ncid);
`

will create a netCDF-4 file with PIO (NC_PIO = NC_UDF0). And this is a general thing that a user-defined format might want to do, since how flags are interpreted and used can be determined by the dispatch layer.

But in order for this to work, the function NC_infermodel() has to check the UDF flags first, and defer handling the rest of the flags to the dispatch layer.

This is an easy fix, but I am also going to modify the test of user-defined formats to check this...






Pull request:
User-defined formats must come first in NC_infermodel, plus test
Fixes #1522 

Also modified tst_udf.c to detect this problem. Also added comments to dinfermodel.c and tst_udf.c to describe what is going on.

@DennisHeimbigner this PR will be of interest to you.

@WardF we absolutely want this in before the next release.

Patch:
diff --git a/libdispatch/dinfermodel.c b/libdispatch/dinfermodel.c
index 5f8b430cc..d4e144f65 100644
--- a/libdispatch/dinfermodel.c
+++ b/libdispatch/dinfermodel.c
@@ -393,7 +393,20 @@ NC_omodeinfer(int useparallel, int cmode, NCmodel* model)
     if(!fIsSet(cmode,NC_FORMAT_ALL))
 	set_default_mode(&cmode);
 
-    /* Process the cmode; may override some already set flags */
+    /* Process the cmode; may override some already set flags. The
+     * user-defined formats must be checked first. They may choose to
+     * use some of the other flags, like NC_NETCDF4, so we must fist
+     * check NC_UDF0 and NC_UDF1 before checking for any other
+     * flag. */
+    if(fIsSet(cmode,(NC_UDF0|NC_UDF1))) {
+	model->format = NC_FORMAT_NETCDF4;
+        if(fIsSet(cmode,NC_UDF0)) {
+	    model->impl = NC_FORMATX_UDF0;
+	} else {
+	    model->impl = NC_FORMATX_UDF1;
+	}
+	goto done;
+    }
 
     if(fIsSet(cmode,NC_64BIT_OFFSET)) {
 	model->impl = NC_FORMATX_NC3;
@@ -416,16 +429,6 @@ NC_omodeinfer(int useparallel, int cmode, NCmodel* model)
         goto done;
     }
 
-    if(fIsSet(cmode,(NC_UDF0|NC_UDF1))) {
-	model->format = NC_FORMAT_NETCDF4;
-        if(fIsSet(cmode,NC_UDF0)) {
-	    model->impl = NC_FORMATX_UDF0;
-	} else {
-	    model->impl = NC_FORMATX_UDF1;
-	}
-	goto done;
-    }
-
     /* Default to classic model */
     model->format = NC_FORMAT_CLASSIC;
     model->impl = NC_FORMATX_NC3;


Test patch:
diff --git a/nc_test4/tst_udf.c b/nc_test4/tst_udf.c
index 25f85f2d1..9d2397d1a 100644
--- a/nc_test4/tst_udf.c
+++ b/nc_test4/tst_udf.c
@@ -184,8 +184,12 @@ main(int argc, char **argv)
          if (nc_open(FILE_NAME, mode[i], &ncid)) ERR;
          if (nc_close(ncid)) ERR;
 
-         /* Open file again and abort, which is the same as closing it. */
-         if (nc_open(FILE_NAME, mode[i], &ncid)) ERR;
+         /* Open file again and abort, which is the same as closing
+          * it. This also tests that the UDF flags are given
+          * priority. If NC_NETCDF4 flag were given priority, then
+          * nc_abort() will not return TEST_VAL_42, but instead will
+          * return 0. */
+         if (nc_open(FILE_NAME, mode[i]|NC_NETCDF4, &ncid)) ERR;
          if (nc_inq_format(ncid, NULL) != TEST_VAL_42) ERR;
          if (nc_inq_format_extended(ncid, NULL, NULL) != TEST_VAL_42) ERR;
          if (nc_abort(ncid) != TEST_VAL_42) ERR;