WIP: Replace dangerous old string functions with less horrible variants, for -fbounds-safety compatibility

.clang-tidy

@@ -1,46 +1,129 @@
+# clang-tidy configuration for nifti_clib.
+#
+# This is a C11 codebase that ships a stable ABI.  Two consequences shape
+# the list below:
+#
+#   * Checks written for C++ idioms are noise here and are turned off
+#     rather than left to accumulate.
+#   * Any check that wants to change linkage or a declaration in an
+#     installed header is off, and must stay off.  See the ABI section.
+#
+# Run it with:
+#   cmake -B build -DCMAKE_EXPORT_COMPILE_COMMANDS=ON
+#   run-clang-tidy -p build -j "$(nproc)" -quiet
+#
 Checks: '-*,
   android-*,
   -android-cloexec-fopen,
+
   bugprone-*,
-  -bugprone-narrowing-conversions,
-  -bugprone-signed-char-misuse,
+  -bugprone-assignment-in-if-condition,
   -bugprone-branch-clone,
   -bugprone-easily-swappable-parameters,
-  -bugprone-not-null-terminated-result,
   -bugprone-implicit-widening-of-multiplication-result,
-  -bugprone-assignment-in-if-condition,
-  -bugprone-macro-parentheses,
+  -bugprone-narrowing-conversions,
+  -bugprone-not-null-terminated-result,
+  -bugprone-signed-char-misuse,
+
   cert-*,
-  -cert-str34-c,
-  -cert-err34-c,
   -cert-err33-c,
+  -cert-err34-c,
   -cert-flp30-c,
+  -cert-str34-c,
+
   clang-analyzer-*,
-  -clang-analyzer-alpha.clone.CloneChecker,
-  -clang-analyzer-alpha.deadcode.UnreachableCode,
-  -clang-analyzer-security.insecureAPI.strcpy,
-  -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
   -clang-analyzer-security.FloatLoopCounter,
+  -clang-analyzer-security.insecureAPI.DeprecatedOrUnsafeBufferHandling,
+  -clang-analyzer-security.insecureAPI.strcpy,
+
   google-*,
-  -google-readability-casting,
   -google-readability-braces-around-statements,
+  -google-readability-casting,
   -google-readability-function-size,
+
   misc-*,
   -misc-confusable-identifiers,
+  -misc-include-cleaner,
   -misc-no-recursion,
+  -misc-use-internal-linkage,
+
   performance-*,
   -performance-type-promotion-in-math-fn,
+
   readability-*,
-  -readability-magic-numbers,
+  -readability-avoid-nested-conditional-operator,
+  -readability-avoid-unconditional-preprocessor-if,
+  -readability-braces-around-statements,
   -readability-else-after-return,
   -readability-identifier-length,
+  -readability-inconsistent-ifelse-braces,
   -readability-isolate-declaration,
-  -readability-braces-around-statements,
+  -readability-magic-numbers,
+  -readability-math-missing-parentheses,
+  -readability-misleading-indentation,
+  -readability-named-parameter,
   -readability-non-const-parameter,
-  -readability-misleading-indentation'
+  -readability-redundant-parentheses'
 
+# ---------------------------------------------------------------------------
+# ABI: checks that must never be enabled
+#
+#   misc-use-internal-linkage    Wants `static` on every function without a
+#     visible external use.  It flags 29 distinct functions here, and 13 of
+#     them are symbols currently exported by libniftiio, libfslio or
+#     libcifti -- FslFileType, FslGetHdrImgNames, fsl_fileexists,
+#     axml_recur_find_xml and the rest.  Applying the fix-it deletes them
+#     from the shared libraries, silently breaking anyone linking against
+#     them.  Most are also the functions -Wmissing-prototypes complains
+#     about; the fix for those is a file-local prototype, not `static`.
+#     Never enable this check without an ABI review.
+#
+#   readability-non-const-parameter    Wants `const` added to pointer
+#     parameters.  Harmless inside a .c file, but its fix-it does not know
+#     which functions are declared in an installed header, and adding const
+#     there changes the published prototype.
+#
+# ---------------------------------------------------------------------------
+# Turned off as noise, with counts from the run that introduced this file
+#
+#   readability-math-missing-parentheses    623.  Wants parentheses around
+#     every * inside a +.  This is a house-style opinion, not a defect
+#     class, and the arithmetic here is conventional.
+#   misc-include-cleaner                    620.  An include-what-you-use
+#     tool.  It flags every transitively-included declaration, including
+#     ones from nifti1_io.h that exist precisely so callers need one header.
+#   readability-inconsistent-ifelse-braces  246.  clang-format owns brace
+#     placement here and is configured with InsertBraces: false.
+#   readability-named-parameter             14.  Omitting parameter names in
+#     a prototype is idiomatic C.
+#   readability-redundant-parentheses       29.  Directly at odds with
+#     bugprone-macro-parentheses, which is enabled and is the one that
+#     catches real defects.
+#
+# ---------------------------------------------------------------------------
+# Notable checks that ARE enabled
+#
+#   bugprone-macro-parentheses              was disabled; re-enabled here.
+#     The macros in nifti_tool.h and the QSTR/NT_FILL families expand their
+#     arguments without parentheses, so any caller passing an expression
+#     gets silent precedence bugs.
+#   bugprone-unchecked-string-to-number-conversion
+#   bugprone-suspicious-realloc-usage        leaks the original block when
+#     realloc returns NULL.
+#   bugprone-misplaced-widening-cast         cast applied after the overflow
+#     rather than before it.
+#   bugprone-multi-level-implicit-pointer-conversion
+#
+# cert-err33-c stays off for now: it flags every unchecked printf and fseek
+# as well as the allocations that matter, which buries the signal.  The
+# allocation cases are covered by clang-analyzer-unix.Malloc instead.
+#
 CheckOptions:
   - key: readability-function-cognitive-complexity.IgnoreMacros
     value: 1
+  # A ratchet, not a target.  Several functions here (nifti_image_read,
+  # nifti_tool's act_* dispatchers) are large by design.  This value is set
+  # just above today's worst so that nothing may get worse; lower it as
+  # functions are split, never raise it.
   - key: readability-function-cognitive-complexity.Threshold
     value: 428

.github/check_workflow_triggers.py

@@ -0,0 +1,110 @@
+#!/usr/bin/env python3
+"""Fail if a workflow's push or pull_request trigger can never match.
+
+A branch filter naming a branch that does not exist leaves the workflow
+configured but never scheduled, which looks identical to a workflow that
+runs and passes: no red check appears, because no check appears at all.
+
+Only filters made entirely of literal names are judged. A filter holding
+any glob is left alone, since whether it can match depends on branches
+that may not exist yet.
+
+  usage: check_workflow_triggers.py [workflow-dir]
+"""
+import glob
+import os
+import subprocess
+import sys
+
+import yaml
+
+GLOB_CHARS = set('*?[]!+@')
+
+
+def existing_branches():
+    """Branch names on the remote, falling back to local refs."""
+    for cmd in (['git', 'ls-remote', '--heads', 'origin'],
+                ['git', 'for-each-ref', '--format=%(refname)', 'refs/heads']):
+        out = subprocess.run(cmd, capture_output=True, text=True)
+        if out.returncode == 0 and out.stdout.strip():
+            names = set()
+            for line in out.stdout.split('\n'):
+                ref = line.split('refs/heads/')[-1].strip()
+                if ref:
+                    names.add(ref)
+            if names:
+                return names
+    return set()
+
+
+def default_branch():
+    env = os.environ.get('DEFAULT_BRANCH')
+    if env:
+        return env.strip()
+    out = subprocess.run(['git', 'symbolic-ref', '--short', 'refs/remotes/origin/HEAD'],
+                         capture_output=True, text=True)
+    if out.returncode == 0 and out.stdout.strip():
+        return out.stdout.strip().split('/')[-1]
+    return None
+
+
+def triggers(doc):
+    """The `on:` mapping. PyYAML reads an unquoted `on` key as True."""
+    for key in (True, 'on', 'On', 'ON'):
+        if isinstance(doc, dict) and key in doc:
+            return doc[key]
+    return None
+
+
+def main(argv):
+    where = argv[1] if len(argv) > 1 else '.github/workflows'
+    branches = existing_branches()
+    default = default_branch()
+    if not branches:
+        print('could not determine the repository branches; nothing checked')
+        return 0
+    print('branches on the remote: %d, default: %s'
+          % (len(branches), default or 'unknown'))
+
+    problems = []
+    for path in sorted(glob.glob(os.path.join(where, '*.yml'))
+                       + glob.glob(os.path.join(where, '*.yaml'))):
+        try:
+            doc = yaml.safe_load(open(path))
+        except yaml.YAMLError as exc:
+            problems.append('%s: cannot parse: %s' % (path, exc))
+            continue
+        on = triggers(doc)
+        if not isinstance(on, dict):
+            continue
+        for event in ('push', 'pull_request', 'pull_request_target'):
+            spec = on.get(event)
+            if not isinstance(spec, dict):
+                continue
+            names = spec.get('branches')
+            if not names:
+                continue
+            if any(GLOB_CHARS & set(n) for n in names):
+                continue
+            live = [n for n in names if n in branches]
+            if not live:
+                problems.append(
+                    '%s: %s.branches names only %s, and no such branch exists.\n'
+                    '    This workflow can never be scheduled.%s'
+                    % (path, event, ', '.join(repr(n) for n in names),
+                       ('\n    The default branch is %r.' % default) if default else ''))
+            elif default and default not in names:
+                print('note: %s: %s.branches does not include the default '
+                      'branch %r' % (path, event, default))
+
+    if problems:
+        print('\nWorkflow trigger check failed:\n')
+        for p in problems:
+            print('  ' + p)
+        return 1
+    print('every push and pull_request trigger can match an existing branch')
+    return 0
+
+
+if __name__ == '__main__':
+    sys.exit(main(sys.argv))

.github/workflows/build.yml

@@ -2,9 +2,9 @@ name: Build and Test
 
 on:
   push:
-    branches: [ main ]
+    branches: [ master ]
   pull_request:
-    branches: [ main ]
+    branches: [ master ]
 
 jobs:
   build:
@@ -68,7 +68,7 @@ jobs:
             shell_tests: "ON"
 
           - name: rel-clang-macos
-            os: macos-11
+            os: macos-latest
             compiler: clang
             scanbuild: ""
             cflags: "-O3 -Wall -Wextra -Wshadow -Wunused-variable -Wunused-parameter -Wunused-function -Wunused -Wno-system-headers -Wno-deprecated -Wwrite-strings"
@@ -86,7 +86,7 @@ jobs:
       AGENT_BUILDDIRECTORY: ${{ github.workspace }}/NIFTIworkspace
       BUILD_SOURCESDIRECTORY: ${{ github.workspace }}
       SYSTEM_PULLREQUEST_SOURCEBRANCH: ${{ github.head_ref }}
-      BUILD_SOURCEBRANCHNAME: ${{ github.head_ref }}
+      BUILD_SOURCEBRANCHNAME: ${{ github.head_ref || github.ref_name }}
       BUILD_BUILDID: ${{ github.run_id }}
       SYSTEM_PULLREQUEST_PULLREQUESTNUMBER: ${{ github.event.pull_request.number }}
       CTEST_SCRIPT_DIRECTORY: ${{ github.workspace }}/cmake
@@ -98,12 +98,12 @@ jobs:
       if: runner.os == 'Linux'
       run: |
         sudo apt-get update
-        sudo apt-get install -y cmake valgrind help2man
+        sudo apt-get install -y cmake valgrind help2man clang-tools
 
     - name: Install Dependencies (macOS)
       if: runner.os == 'macOS'
       run: |
-        brew install cmake sed help2man
+        brew install cmake gnu-sed help2man
 
     - name: Set Compiler and Flags
       run: |
@@ -131,5 +131,5 @@ jobs:
 
     - name: Run CTest
       run: |
-        eval $SCANBUILD_EXE ctest -S ${CTEST_SCRIPT_DIRECTORY}/travis_dashboard.cmake -V -j 4
+        eval $SCANBUILD_EXE ctest -S ${CTEST_SCRIPT_DIRECTORY}/github_dashboard.cmake -V -j 4