Make sure mode flags are properly defined in netcdf.h

RELEASE_NOTES.md

@@ -7,6 +7,7 @@ This file contains a high-level description of this package's evolution. Release
 
 ## 4.8.2 - TBD
 
+* [Bug Fix] Make sure that netcdf.h accurately defines the flags in the open/create mode flags. See [Github #2183](https://github.com/Unidata/netcdf-c/pull/2183).
 * [Enhancement] Improve support for msys2+mingw platform. See [Github #2171](https://github.com/Unidata/netcdf-c/pull/2171).
 * [Bug Fix] Clean up the various inter-test dependencies in ncdump for CMake. See [Github #2168](https://github.com/Unidata/netcdf-c/pull/2168).
 * [Enhancement] Added options to suppress the new behavior from [Github #2135](https://github.com/Unidata/netcdf-c/pull/2135).  The options for `cmake` and `configure` are, respectively `-DENABLE_LIBXML2` and `--(enable/disable)-libxml2`. Both of these options defaul to 'on/enabled'.  When disabled, the bundled `ezxml` XML interpreter is used regardless of whether `libxml2` is present on the system. 

include/nc3internal.h

@@ -224,15 +224,18 @@ NC_lookupvar(NC3_INFO* ncp, int varid, NC_var **varp);
 struct NC3_INFO {
     /* contains the previous NC during redef. */
     NC3_INFO *old;
-    /* flags */
-#define NC_CREAT 2      /* in create phase, cleared by ncendef */
-#define NC_INDEF 8      /* in define mode, cleared by ncendef */
-#define NC_NSYNC 0x10   /* synchronise numrecs on change */
-#define NC_HSYNC 0x20   /* synchronise whole header on change */
-#define NC_NDIRTY 0x40  /* numrecs has changed */
-#define NC_HDIRTY 0x80  /* header info has changed */
-/*      NC_NOFILL in netcdf.h, historical interface */
-    int flags;
+    int flags; /* mode flags */
+    int state; /* state transitions flags */
+#	define NC_CREAT 0x1      /* in create phase, cleared by ncendef */
+#	define NC_INDEF 0x2      /* in define mode, cleared by ncendef */
+#	define NC_NSYNC 0x4      /* synchronise numrecs on change */
+#	define NC_HSYNC 0x8      /* synchronise whole header on change */
+#	define NC_NDIRTY 0x10  /* numrecs has changed */
+#	define NC_HDIRTY 0x20  /* header info has changed */
+/* NC_NOFILL defined in netcdf.h, historical interface */
+#if 0
+#	define NC_NOFILL 0x100   /**< Argument to nc_set_fill() to turn off filling of data. */
+#endif
     struct ncio* nciop;
     size_t chunk;   /* largest extent this layer will request from ncio->get() */
     size_t xsz;     /* external size of this header, == var[0].begin */
@@ -258,31 +261,31 @@ struct NC3_INFO {
     fClr((ncp)->flags, NC_WRITE)
 
 #define NC_IsNew(ncp)                           \
-    fIsSet((ncp)->flags, NC_CREAT)
+    fIsSet((ncp)->state, NC_CREAT)
 
 #define NC_indef(ncp)                                   \
-    (NC_IsNew(ncp) || fIsSet((ncp)->flags, NC_INDEF))
+    (NC_IsNew(ncp) || fIsSet((ncp)->state, NC_INDEF))
 
 #define set_NC_ndirty(ncp)                      \
-    fSet((ncp)->flags, NC_NDIRTY)
+    fSet((ncp)->state, NC_NDIRTY)
 
 #define NC_ndirty(ncp)                          \
-    fIsSet((ncp)->flags, NC_NDIRTY)
+    fIsSet((ncp)->state, NC_NDIRTY)
 
 #define set_NC_hdirty(ncp)                      \
-    fSet((ncp)->flags, NC_HDIRTY)
+    fSet((ncp)->state, NC_HDIRTY)
 
 #define NC_hdirty(ncp)                          \
-    fIsSet((ncp)->flags, NC_HDIRTY)
+    fIsSet((ncp)->state, NC_HDIRTY)
 
 #define NC_dofill(ncp)                          \
-    (!fIsSet((ncp)->flags, NC_NOFILL))
+    (!fIsSet((ncp)->state, NC_NOFILL))
 
 #define NC_doHsync(ncp)                         \
-    fIsSet((ncp)->flags, NC_HSYNC)
+    fIsSet((ncp)->state, NC_HSYNC)
 
 #define NC_doNsync(ncp)                         \
-    fIsSet((ncp)->flags, NC_NSYNC)
+    fIsSet((ncp)->state, NC_NSYNC)
 
 #  define NC_get_numrecs(nc3i)                  \
     ((nc3i)->numrecs)

include/nc4internal.h

@@ -276,29 +276,23 @@ typedef struct NC_GRP_INFO
     NCindex* vars;               /**< NCindex<NC_VAR_INFO_T> * */
 } NC_GRP_INFO_T;
 
-/* These constants apply to the cmode parameter in the
+/* These constants apply to the flags field in the
  * HDF5_FILE_INFO_T defined below. */
-/* Make sure they do not conflict with defined flags in netcdf.h */
-#define NC_CREAT 0x10002      /**< in create phase, cleared by ncendef */
-#define NC_INDEF 0x10008      /**< in define mode, cleared by ncendef */
-#define NC_NSYNC 0x10010   /**< synchronise numrecs on change */
-#define NC_HSYNC 0x10020   /**< synchronise whole header on change */
-#define NC_NDIRTY 0x10040  /**< numrecs has changed */
-#define NC_HDIRTY 0x10080  /**< header info has changed */
+#define NC_INDEF  0x01  /**< in define mode, cleared by ncendef */
 
 /** This is the metadata we need to keep track of for each
   * netcdf-4/HDF5 file. */
 
-typedef struct  NC_FILE_INFO
+typedef struct NC_FILE_INFO
 {
     NC_OBJ hdr;
     NC *controller; /**< Pointer to containing NC. */
 #ifdef USE_PARALLEL4
     MPI_Comm comm;  /**< Copy of MPI Communicator used to open the file. */
     MPI_Info info;  /**< Copy of MPI Information Object used to open the file. */
 #endif
-    int flags;      /**< Flags used to open the file. */
-    int cmode;      /**< Create mode used to create the file. */
+    int cmode;      /**< Create/Open mode for the file. */
+    int flags;      /**< State transition flags . */
     nc_bool_t parallel;   /**< True if file is open for parallel access */
     nc_bool_t redef;      /**< True if redefining an existing file */
     nc_bool_t no_attr_create_order; /**< True if the creation order tracking of attributes is disabled (netcdf-4 only) */

include/netcdf.h

@@ -115,13 +115,14 @@ extern "C" {
 #define NC_NOFILL       0x100   /**< Argument to nc_set_fill() to turn off filling of data. */
 
 /* Define the ioflags bits for nc_create and nc_open.
-   currently unused:
+   Currently unused in lower 16 bits:
         0x0002
-   and the whole upper 16 bits
-   Note: nc4internal also defines flags in this space even tho it should not.
-   so check there around #define NC_CREAT.
+   All upper 16 bits are unused except
+        0x20000
 */
 
+/* Lower 16 bits */
+
 #define NC_NOWRITE       0x0000 /**< Set read-only access for nc_open(). */
 #define NC_WRITE         0x0001 /**< Set read-write access for nc_open(). */
 
@@ -161,7 +162,8 @@ Use this in mode flags for both nc_create() and nc_open(). */
 #define NC_PERSIST       0x4000  /**< Save diskless contents to disk. Mode flag for nc_open() or nc_create() */
 #define NC_INMEMORY      0x8000  /**< Read from memory. Mode flag for nc_open() or nc_create() */
 
-#define NC_NOATTCREORD   0x20000 /**< Disable the netcdf-4 (hdf5) attribute creation order tracking */
+/* Upper 16 bits */
+#define NC_NOATTCREORD  0x20000 /**< Disable the netcdf-4 (hdf5) attribute creation order tracking */
 
 #define NC_MAX_MAGIC_NUMBER_LEN 8 /**< Max len of user-defined format magic number. */
 

libsrc/nc3internal.c

@@ -413,7 +413,7 @@ write_numrecs(NC3_INFO *ncp)
 	(void) ncio_rel(ncp->nciop, NC_NUMRECS_OFFSET, RGN_MODIFIED);
 
 	if(status == NC_NOERR)
-		fClr(ncp->flags, NC_NDIRTY);
+		fClr(ncp->state, NC_NDIRTY);
 
 	return status;
 }
@@ -435,7 +435,7 @@ read_NC(NC3_INFO *ncp)
 	status = nc_get_NC(ncp);
 
 	if(status == NC_NOERR)
-		fClr(ncp->flags, NC_NDIRTY | NC_HDIRTY);
+		fClr(ncp->state, NC_NDIRTY | NC_HDIRTY);
 
 	return status;
 }
@@ -454,7 +454,7 @@ write_NC(NC3_INFO *ncp)
 	status = ncx_put_NC(ncp, NULL, 0, 0);
 
 	if(status == NC_NOERR)
-		fClr(ncp->flags, NC_NDIRTY | NC_HDIRTY);
+		fClr(ncp->state, NC_NDIRTY | NC_HDIRTY);
 
 	return status;
 }
@@ -864,7 +864,7 @@ NC_endef(NC3_INFO *ncp,
 	{
 		/* a plain redef, not a create */
 		assert(!NC_IsNew(ncp));
-		assert(fIsSet(ncp->flags, NC_INDEF));
+		assert(fIsSet(ncp->state, NC_INDEF));
 		assert(ncp->begin_rec >= ncp->old->begin_rec);
 		assert(ncp->begin_var >= ncp->old->begin_var);
 
@@ -937,7 +937,7 @@ NC_endef(NC3_INFO *ncp,
 		ncp->old = NULL;
 	}
 
-	fClr(ncp->flags, NC_CREAT | NC_INDEF);
+	fClr(ncp->state, NC_CREAT | NC_INDEF);
 
 	return ncio_sync(ncp->nciop);
 }
@@ -1071,7 +1071,7 @@ NC3_create(const char *path, int ioflags, size_t initialsz, int basepe,
 		goto unwind_alloc;
 	}
 
-	fSet(nc3->flags, NC_CREAT);
+	fSet(nc3->state, NC_CREAT);
 
 	if(fIsSet(nc3->nciop->ioflags, NC_SHARE))
 	{
@@ -1082,7 +1082,7 @@ NC3_create(const char *path, int ioflags, size_t initialsz, int basepe,
 		 * automatically.  Some sort of IPC (external to this package)
 		 * would be used to trigger a call to nc_sync().
 		 */
-		fSet(nc3->flags, NC_NSYNC);
+		fSet(nc3->state, NC_NSYNC);
 	}
 
 	status = ncx_put_NC(nc3, &xp, sizeof_off_t, nc3->xsz);
@@ -1173,29 +1173,12 @@ NC3_open(const char *path, int ioflags, int basepe, size_t *chunksizehintp,
             goto unwind_alloc;
         }
 
-#ifdef ENABLE_BYTERANGE
-	{
-	    NCURI* uri = NULL;
-	    ncuriparse(path,&uri);
-	    if(uri) {
-		/* If the model specified the use of byte-ranges, then signal by
-		a temporary hack using one of the flags in the ioflags. */
-		if(NC_testmode(uri,"bytes")) {
-#  ifdef ENABLE_S3_SDK
-		    if(NC_iss3(uri)) ioflags |= NC_S3SDK; else
-#  endif
-			ioflags |= NC_HTTP;
-		}
-		ncurifree(uri);
-	    }
-	}
-#endif /*ENABLE_BYTERANGE*/
         status = ncio_open(path, ioflags, 0, 0, &nc3->chunk, parameters,
 			       &nc3->nciop, NULL);
 	if(status)
 		goto unwind_alloc;
 
-	assert(nc3->flags == 0);
+	assert(nc3->state == 0);
 
 	if(fIsSet(nc3->nciop->ioflags, NC_SHARE))
 	{
@@ -1206,7 +1189,7 @@ NC3_open(const char *path, int ioflags, int basepe, size_t *chunksizehintp,
 		 * automatically.  Some sort of IPC (external to this package)
 		 * would be used to trigger a call to nc_sync().
 		 */
-		fSet(nc3->flags, NC_NSYNC);
+		fSet(nc3->state, NC_NSYNC);
 	}
 
 	status = nc_get_NC(nc3);
@@ -1279,10 +1262,10 @@ NC3_abort(int ncid)
 	{
 		/* a plain redef, not a create */
 		assert(!NC_IsNew(nc3));
-		assert(fIsSet(nc3->flags, NC_INDEF));
+		assert(fIsSet(nc3->state, NC_INDEF));
 		free_NC3INFO(nc3->old);
 		nc3->old = NULL;
-		fClr(nc3->flags, NC_INDEF);
+		fClr(nc3->state, NC_INDEF);
 	}
 	else if(!NC_readonly(nc3))
 	{
@@ -1397,7 +1380,7 @@ NC3_redef(int ncid)
 	if(nc3->old == NULL)
 		return NC_ENOMEM;
 
-	fSet(nc3->flags, NC_INDEF);
+	fSet(nc3->state, NC_INDEF);
 
 	return NC_NOERR;
 }
@@ -1509,15 +1492,15 @@ NC3_set_fill(int ncid,
 	if(NC_readonly(nc3))
 		return NC_EPERM;
 
-	oldmode = fIsSet(nc3->flags, NC_NOFILL) ? NC_NOFILL : NC_FILL;
+	oldmode = fIsSet(nc3->state, NC_NOFILL) ? NC_NOFILL : NC_FILL;
 
 	if(fillmode == NC_NOFILL)
 	{
-		fSet(nc3->flags, NC_NOFILL);
+		fSet(nc3->state, NC_NOFILL);
 	}
 	else if(fillmode == NC_FILL)
 	{
-		if(fIsSet(nc3->flags, NC_NOFILL))
+		if(fIsSet(nc3->state, NC_NOFILL))
 		{
 			/*
 			 * We are changing back to fill mode
@@ -1527,7 +1510,7 @@ NC3_set_fill(int ncid,
 			if(status != NC_NOERR)
 				return status;
 		}
-		fClr(nc3->flags, NC_NOFILL);
+		fClr(nc3->state, NC_NOFILL);
 	}
 	else
 	{

libsrc/ncio.c

@@ -12,6 +12,8 @@
 #include "netcdf.h"
 #include "ncio.h"
 #include "fbits.h"
+#include "ncuri.h"
+#include "ncrc.h"
 
 /* With the advent of diskless io, we need to provide
    for multiple ncio packages at the same time,
@@ -46,6 +48,9 @@ extern int ffio_open(const char*,int,off_t,size_t,size_t*,void*,ncio**,void** co
      extern int memio_create(const char*,int,size_t,off_t,size_t,size_t*,void*,ncio**,void** const);
      extern int memio_open(const char*,int,off_t,size_t,size_t*,void*,ncio**,void** const);
 
+/* Forward */
+static int urlmodetest(const char* path);
+
 int
 ncio_create(const char *path, int ioflags, size_t initialsz,
                        off_t igeto, size_t igetsz, size_t *sizehintp,
@@ -78,6 +83,8 @@ ncio_open(const char *path, int ioflags,
 		     void* parameters,
                      ncio** iopp, void** const mempp)
 {
+    int modetest = urlmodetest(path);
+
     /* Diskless open has the following constraints:
        1. file must be classic version 1 or 2 or 5
      */
@@ -93,12 +100,11 @@ ncio_open(const char *path, int ioflags,
     }
 #  endif /*USE_MMAP*/
 #  ifdef ENABLE_BYTERANGE
-   /* The NC_HTTP flag is a big hack until we can reorganize the ncio interface */
-   if(fIsSet(ioflags,NC_HTTP)) {
+   if(modetest == NC_HTTP) {
         return httpio_open(path,ioflags,igeto,igetsz,sizehintp,parameters,iopp,mempp);
    }
 #  ifdef ENABLE_S3_SDK
-   if(fIsSet(ioflags,NC_S3SDK)) {
+   if(modetest == NC_S3SDK) {
        return s3io_open(path,ioflags,igeto,igetsz,sizehintp,parameters,iopp,mempp);
    }
 #  endif
@@ -162,3 +168,29 @@ ncio_close(ncio* const nciop, int doUnlink)
     int status = nciop->close(nciop,doUnlink);
     return status;
 }
+
+/* URL utilities */
+
+/*
+Check mode flags and return:
+NC_HTTP => byterange
+NC_S3SDK => s3
+0 => Not URL
+*/
+static int
+urlmodetest(const char* path)
+{
+    int kind = 0;
+    NCURI* uri = NULL;
+
+    ncuriparse(path,&uri);
+    if(uri == NULL) return 0; /* Not URL */
+    if(NC_testmode(uri, "bytes"))
+        kind = NC_HTTP;
+    else if(NC_testmode(uri, "s3"))
+        kind = NC_S3SDK;
+    else
+        kind = 0;
+    ncurifree(uri);
+    return kind;
+}

libsrc/ncio.h

@@ -11,11 +11,9 @@
 #include <sys/types.h>	/* off_t */
 #include "netcdf.h"
 
-/* Define internal use only flags to signal use of byte ranges and S3.
-   This is temporary until we can re-organize the ncio open/create API.
-*/
-#define NC_HTTP  0x80000000
-#define NC_S3SDK 0x40000000
+/* Define internal use only flags to signal use of byte ranges and S3. */
+#define NC_HTTP  1
+#define NC_S3SDK 2
 
 typedef struct ncio ncio;	/* forward reference */
 

libsrc/s3io.c

@@ -36,6 +36,7 @@
 #include "fbits.h"
 #include "rnd.h"
 #include "ncs3sdk.h"
+#include "ncuri.h"
 
 #define DEFAULTPAGESIZE 16384