Re-promote some GNU warnings to errors (#1508)

* Promote some GCC warnings back to errors and deal with build breakage.
I tested configuring and building with GCC 8.3.0 with

`.../configure --prefix=$HOME/wrk/install/manydsets --enable-build-mode=production --enable-symbols=-gdwarf-4 --enable-warnings-as-errors --enable-java --enable-hl --enable-fortran --enable-cxx --enable-mirror-vfd --enable-ros3-vfd --enable-direct-vfd`

and with the same options but `--enable-build-mode=debug`.

* Promote -Wrestrict to -Werror=restrict to help catch overlapping
arguments to strcpy, memcpy, et cetera, at compile time.

* NFCI: sort lines.

* Committing clang-format changes

* NFCI: remove whitespace at EOL.

* Re-use temporary variable `shared`.

* Remove merge oopsie.

* Mention that no op codes are understood per review comment.

* Change this back to the develop branch's way, since this change isn't
integral to the PR.

* Committing clang-format changes

* NFCI: reduce differences with `develop` branch in the hope that that
also kicks off CI.

Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Dana Robinson <43805+derobins@users.noreply.github.com>

8 files changed, 20 insertions, 60 deletions

diff --git a/config/cmake/HDFCompilerFlags.cmake b/config/cmake/HDFCompilerFlags.cmake
index eb43b73..61218dc 100644
--- a/config/cmake/HDFCompilerFlags.cmake
+++ b/config/cmake/HDFCompilerFlags.cmake
@@ -231,6 +231,7 @@ if (CMAKE_C_COMPILER_ID STREQUAL "GNU")
   # Append more extra warning flags that only gcc 7.x+ knows about
   if (NOT CMAKE_C_COMPILER_VERSION VERSION_LESS 7.0)
     ADD_H5_FLAGS (H5_CFLAGS "${HDF5_SOURCE_DIR}/config/gnu-warnings/7")
+    ADD_H5_FLAGS (H5_CFLAGS "${HDF5_SOURCE_DIR}/config/gnu-warnings/error-7")
     if (HDF5_ENABLE_DEV_WARNINGS)
       ADD_H5_FLAGS (H5_CFLAGS "${HDF5_SOURCE_DIR}/config/gnu-warnings/developer-7")
     #else ()
diff --git a/config/gnu-flags b/config/gnu-flags
index fe6626d..e25f59b 100644
--- a/config/gnu-flags
+++ b/config/gnu-flags
@@ -254,6 +254,7 @@ if test "X-gcc" = "X-$cc_vendor"; then
     # gcc >= 7
     if test $cc_vers_major -ge 7; then
         H5_CFLAGS="$H5_CFLAGS $(load_gnu_arguments 7)"
+        H5_ECFLAGS="$H5_ECFLAGS $(load_gnu_arguments error-7)"
         DEVELOPER_WARNING_CFLAGS="$DEVELOPER_WARNING_CFLAGS $(load_gnu_arguments developer-7)"
     fi
 
diff --git a/config/gnu-warnings/7 b/config/gnu-warnings/7
index 9d5b44d..cb8834a 100644
--- a/config/gnu-warnings/7
+++ b/config/gnu-warnings/7
@@ -3,4 +3,3 @@
 -Wduplicated-branches
 -Wformat-overflow=2
 -Wformat-truncation=1
--Wrestrict
diff --git a/config/gnu-warnings/error-7 b/config/gnu-warnings/error-7
new file mode 100644
index 0000000..8eb9fe4
--- /dev/null
+++ b/config/gnu-warnings/error-7
@@ -0,0 +1 @@
+-Werror=restrict
diff --git a/config/gnu-warnings/error-8 b/config/gnu-warnings/error-8
index cbb25f6..36c1414 100644
--- a/config/gnu-warnings/error-8
+++ b/config/gnu-warnings/error-8
@@ -6,17 +6,4 @@
 # in order to detect initializations that occur there.  It's possible
 # that GCC 8 only performs that analysis at -O3, though.
 #
-#
-# NOTE: File Driver files are not compatible with these warnings as errors
-#     H5FDlog.c,
-#       -Werror=maybe-uninitialized
--Wmaybe-uninitialized
-# NOTE: src/ files are not compatible with these warnings as errors
-#     src/H5Shyper.c,src/H5SL.c,src/H5Shyper.c
-#       -Werror=maybe-uninitialized
-# NOTE: Test files are not compatible with these warnings as errors
-#     test/cache_common.c,
-#       -Werror=maybe-uninitialized
-# NOTE: hl/src/ files are not compatible with these warnings as errors
-#     hl/src/H5DS.c,
-#       -Werror=maybe-uninitialized
+-Werror=maybe-uninitialized
diff --git a/config/gnu-warnings/error-general b/config/gnu-warnings/error-general
index a66d284..73d1dd5 100644
--- a/config/gnu-warnings/error-general
+++ b/config/gnu-warnings/error-general
@@ -3,7 +3,10 @@
 # circumstances, so ask the compiler to treat them as errors:
 #
 -Werror=bad-function-cast
+-Werror=cast-align
+-Werror=format
 -Werror=implicit-function-declaration
+-Werror=int-to-pointer-cast
 -Werror=missing-declarations
 -Werror=missing-prototypes
 -Werror=nested-externs
@@ -11,10 +14,11 @@
 -Werror=packed
 -Werror=pointer-sign
 -Werror=pointer-to-int-cast
--Werror=int-to-pointer-cast
 -Werror=redundant-decls
 -Werror=strict-prototypes
 -Werror=switch
+-Werror=unused-but-set-variable
+-Werror=unused-variable
 #
 #-Werror=discarded-qualifiers
 #
@@ -25,43 +29,11 @@
 #
 -Wunused-function
 #
-#     H5FDdrvr_module.h
-#       -Werror=unused-variable
-#
--Wunused-variable
-#
 #     H5VLpassthru.c
 #       -Werror=unused-parameter
 #
 -Wunused-parameter
 #
-#
-#
-# NOTE: Tools files are not compatible with these warnings as errors
-#     lib/h5tools.c
-#       -Werror=cast-align
-#
--Wcast-align
-#
-#     lib/h5diff_array.c
-#       -Werror=unused-but-set-variable
-#
--Wunused-but-set-variable
-#
-#     lib/h5tools_utils.c
-#       -Werror=unused-parameter
-#
-#
-# NOTE: JNI files are not compatible with these warnings as errors
-#     jni/h5pDCPLImp.c,jni/nativeData.c,jni/h5util.c,jni/h5rImp.c
-#     jni/h5sImp.c,jni/h5tImp.c
-#       -Werror=cast-align
-#     jni/h5util.c
-#       -Werror=format(-overflow)
-#
--Werror=format
-#
-#
 #Examples and tests do not use the same set of extensive warning flags as libraries
 # Here is a list of tests and examples that have issues with the stricter warnings as error
 #
diff --git a/config/ibm-aix b/config/ibm-aix
index b4a558d..f48f9d8 100644
--- a/config/ibm-aix
+++ b/config/ibm-aix
@@ -47,10 +47,10 @@ if test "X-" = "X-$FC"; then
   fi
 fi
 
-# While we try to avoid setting FCFLAGS directly for use in compilation, in 
-# this case we need the -k flag present for some configure checks. As such, 
-# the configure script saves the user's set FCFLAGS before running, and 
-# restores them when complete. We must then set up both FCFLAGS and H5_FCFLAGS 
+# While we try to avoid setting FCFLAGS directly for use in compilation, in
+# this case we need the -k flag present for some configure checks. As such,
+# the configure script saves the user's set FCFLAGS before running, and
+# restores them when complete. We must then set up both FCFLAGS and H5_FCFLAGS
 # to ensure the flag is present for both configure as well as for the build.
 if test "X-" = "X-$f9x_flags_set"; then
   F9XSUFFIXFLAG="-qsuffix=f=f90"
diff --git a/src/H5FDsec2.c b/src/H5FDsec2.c
index aabee0f..acc705e 100644
--- a/src/H5FDsec2.c
+++ b/src/H5FDsec2.c
@@ -1075,20 +1075,19 @@ done:
  *-------------------------------------------------------------------------
  */
 static herr_t
-H5FD__sec2_ctl(H5FD_t H5_ATTR_UNUSED *_file, uint64_t op_code, uint64_t flags,
+H5FD__sec2_ctl(H5FD_t *_file, uint64_t H5_ATTR_UNUSED op_code, uint64_t flags,
                const void H5_ATTR_UNUSED *input, void H5_ATTR_UNUSED **output)
 {
     herr_t ret_value = SUCCEED;
 
     FUNC_ENTER_PACKAGE
 
-    switch (op_code) {
-        /* Unknown op code */
-        default:
-            if (flags & H5FD_CTL__FAIL_IF_UNKNOWN_FLAG)
-                HGOTO_ERROR(H5E_VFL, H5E_FCNTL, FAIL, "unknown op_code and fail if unknown flag is set")
-            break;
-    }
+    /* Sanity checks */
+    HDassert(_file);
+
+    /* No op codes are understood. */
+    if (flags & H5FD_CTL__FAIL_IF_UNKNOWN_FLAG)
+        HGOTO_ERROR(H5E_VFL, H5E_FCNTL, FAIL, "unknown op_code and fail if unknown flag is set")
 
 done:
     FUNC_LEAVE_NOAPI(ret_value)