diff --git a/.clang-tidy b/.clang-tidy index f2e41d16..7a418c12 100644 --- a/.clang-tidy +++ b/.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 diff --git a/.github/check_workflow_triggers.py b/.github/check_workflow_triggers.py new file mode 100755 index 00000000..8e9bceb6 --- /dev/null +++ b/.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)) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 4c82448d..3bb63fbe 100644 --- a/.github/workflows/build.yml +++ b/.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 diff --git a/.github/workflows/cmake-multi-platform.yml b/.github/workflows/cmake-multi-platform.yml index 4a19f9a3..3b577d0a 100644 --- a/.github/workflows/cmake-multi-platform.yml +++ b/.github/workflows/cmake-multi-platform.yml @@ -1,5 +1,30 @@ -# This starter workflow is for a CMake project running on multiple platforms. There is a different starter workflow if you just want a single platform. -# See: https://github.com/actions/starter-workflows/blob/main/ci/cmake-single-platform.yml +# Per-PR build and test matrix. +# +# Axes and why each one is here: +# +# os / c_compiler GNU, Clang and AppleClang. macOS has no gcc entry +# because /usr/bin/gcc there is a clang shim that +# reports AppleClang, so a macos+gcc leg duplicates +# the macos+clang leg exactly. +# shared BUILD_SHARED_LIBS also gates TEST_INSTALL, so the +# ON legs are the only ones that run install_linking +# and therefore the only ones covering the install +# and export path. +# fslstyle -DFSLSTYLE is not additive. It rewrites behaviour +# in niftilib and nifti2 through global -DFSLSTYLE, +# -DPIGZ and -DREJECT_COMPLEX definitions, and one of +# them has an #else that changes the pixdim[0] value +# written to disk (nifti1_io.c). Both values are +# real configurations and neither subsumes the other. +# build_type Debug turns assert() from a no-op into an abort, +# and the library has live assert() sites. One +# static Debug leg carries this; Debug is not a +# second pass over the other axes. +# +# The optional libraries (cifti, fsliolib) are built everywhere rather +# than being an axis of their own; they are additive, so ON covers OFF. +# The minimal-configuration job below covers the other direction. + name: CMake on multiple platforms on: @@ -10,63 +35,258 @@ on: jobs: build: + name: ${{ matrix.os }} ${{ matrix.c_compiler }} ${{ matrix.build_type }} shared=${{ matrix.shared }} fslstyle=${{ matrix.fslstyle }} runs-on: ${{ matrix.os }} strategy: - # Set fail-fast to false to ensure that feedback is delivered for all matrix combinations. Consider changing this to true when your workflow is stable. fail-fast: false - - # Set up a matrix to run the following 3 configurations: - # 1. - # 2. - # 3. - # - # To add more build types (Release, Debug, RelWithDebInfo, etc.) customize the build_type list. matrix: os: [ubuntu-latest, macos-latest] - build_type: [Release] c_compiler: [gcc, clang] + build_type: [Release] + shared: ["OFF", "ON"] + fslstyle: ["OFF", "ON"] include: - - os: macos-latest - c_compiler: clang - cpp_compiler: clang++ - os: ubuntu-latest c_compiler: gcc cpp_compiler: g++ - os: ubuntu-latest c_compiler: clang cpp_compiler: clang++ - exclude: + - os: macos-latest + c_compiler: clang + cpp_compiler: clang++ + # One Debug leg, static. Debug exists to make assert() + # live, not to re-cover the linkage or platform axes, so a + # single configuration carries it. FSLSTYLE is ON so the + # assert() sites behind those defines are reachable too. - os: ubuntu-latest - c_compiler: cl + c_compiler: gcc + cpp_compiler: g++ + build_type: Debug + shared: "OFF" + fslstyle: "ON" + exclude: + # /usr/bin/gcc on macOS is a clang shim; this leg would be an + # exact duplicate of the macos clang leg. + - os: macos-latest + c_compiler: gcc steps: - uses: actions/checkout@v4 + - name: Install optional dependencies + if: runner.os == 'Linux' + run: sudo apt-get update && sudo apt-get install -y libexpat1-dev zlib1g-dev + - name: Set reusable strings - # Turn repeated input strings (such as the build output directory) into step outputs. These step outputs can be used throughout the workflow file. id: strings shell: bash - run: | - echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT" + run: echo "build-output-dir=${{ github.workspace }}/build" >> "$GITHUB_OUTPUT" - name: Configure CMake - # Configure CMake in a 'build' subdirectory. `CMAKE_BUILD_TYPE` is only required if you are using a single-configuration generator such as make. - # See https://cmake.org/cmake/help/latest/variable/CMAKE_BUILD_TYPE.html?highlight=cmake_build_type run: > cmake -B ${{ steps.strings.outputs.build-output-dir }} -DCMAKE_CXX_COMPILER=${{ matrix.cpp_compiler }} -DCMAKE_C_COMPILER=${{ matrix.c_compiler }} -DCMAKE_BUILD_TYPE=${{ matrix.build_type }} + -DBUILD_SHARED_LIBS=${{ matrix.shared }} + -DUSE_CIFTI_CODE=ON + -DUSE_FSL_CODE=ON + -DFSLSTYLE=${{ matrix.fslstyle }} -S ${{ github.workspace }} - name: Build - # Build your program with the given configuration. Note that --config is needed because the default Windows generator is a multi-config generator (Visual Studio generator). run: cmake --build ${{ steps.strings.outputs.build-output-dir }} --config ${{ matrix.build_type }} - name: Test working-directory: ${{ steps.strings.outputs.build-output-dir }} - # Execute tests defined by the CMake configuration. Note that --build-config is needed because the default Windows generator is a multi-config generator (Visual Studio generator). - # See https://cmake.org/cmake/help/latest/manual/ctest.1.html for more detail - run: ctest --build-config ${{ matrix.build_type }} + run: ctest --build-config ${{ matrix.build_type }} --output-on-failure + + minimal: + # The configuration a downstream project vendoring only the core + # reader selects. Nothing else in the matrix proves the tree still + # configures with the optional subdirectories switched off. + name: minimal configuration (znzlib + niftilib only) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Configure CMake + run: > + cmake -B ${{ github.workspace }}/build + -DCMAKE_BUILD_TYPE=Release + -DUSE_NIFTI2_CODE=OFF + -DUSE_NIFTICDF_CODE=OFF + -DUSE_CIFTI_CODE=OFF + -DUSE_FSL_CODE=OFF + -DNIFTI_BUILD_APPLICATIONS=OFF + -S ${{ github.workspace }} + - name: Build + run: cmake --build ${{ github.workspace }}/build + - name: Test + working-directory: ${{ github.workspace }}/build + # This configuration registers no tests at all, because the test + # programs live with the applications that are switched off here. + # --no-tests=ignore keeps that from being an error; the value of + # this job is that the tree still configures and builds. + run: ctest --output-on-failure --no-tests=ignore + oldest-cmake: + # Guards cmake_minimum_required. Every other runner carries a + # recent CMake, so a policy or command that needs a newer release + # than the project claims to support is invisible to them. + # PR #30 was exactly that: CMP0169 does not exist before 3.30 and + # setting it unconditionally was fatal on Ubuntu 24.04's 3.28. + name: oldest supported CMake + runs-on: ubuntu-24.04 + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y cmake ninja-build libexpat1-dev zlib1g-dev + - name: Show CMake version + run: cmake --version + - name: Configure CMake + run: > + cmake -G Ninja -B ${{ github.workspace }}/build + -DCMAKE_BUILD_TYPE=Release + -DUSE_CIFTI_CODE=ON + -DUSE_FSL_CODE=ON + -S ${{ github.workspace }} + - name: Build + run: cmake --build ${{ github.workspace }}/build + - name: Test + working-directory: ${{ github.workspace }}/build + run: ctest --output-on-failure + + exported-symbols: + # The exported symbol set is the ABI. Several open changes assert + # that they leave it untouched and nothing verified that claim, so + # this job diffs it against a committed baseline. An intentional + # change to the ABI updates the baseline in the same commit, which + # makes the change visible in review rather than implicit. + name: exported symbol baseline + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y libexpat1-dev zlib1g-dev ninja-build + - name: Configure CMake + run: > + cmake -G Ninja -B ${{ github.workspace }}/build + -DCMAKE_BUILD_TYPE=Release + -DBUILD_SHARED_LIBS=ON + -DUSE_CIFTI_CODE=ON + -DUSE_FSL_CODE=ON + -S ${{ github.workspace }} + - name: Build + run: cmake --build ${{ github.workspace }}/build + - name: Collect exported symbols + run: | + bash cmake/collect_exported_symbols.sh \ + "${{ github.workspace }}/build" \ + /tmp/exported_symbols.txt + - name: Compare against the committed baseline + run: | + if ! diff -u cmake/exported_symbols_linux.txt /tmp/exported_symbols.txt; then + echo "" + echo "The exported symbol set changed." + echo "If that is intended, update cmake/exported_symbols_linux.txt" + echo "in this commit so the ABI change is visible in review:" + echo " bash cmake/collect_exported_symbols.sh cmake/exported_symbols_linux.txt" + exit 1 + fi + echo "Exported symbols match the baseline." + + missing-declarations: + # A function defined without a prior declaration is either missing + # from its header or should have been static. Both shapes reached + # master before, because the flag is in the project's clean set but + # nothing makes it fatal, and a warning in a build that passes is + # not read. + # + # Promoting this one flag needs no preparatory cleanup. The whole + # NIFTI_WARNINGS_AS_ERRORS set is still blocked by + # -Wmaybe-uninitialized in fslio.c, and waiting for that would + # leave this uncovered meanwhile. + name: no undeclared external functions (FSLSTYLE=${{ matrix.fslstyle }}) + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + fslstyle: ["OFF", "ON"] + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y libexpat1-dev zlib1g-dev ninja-build + - name: Configure CMake + run: > + cmake -G Ninja -B ${{ github.workspace }}/build + -DCMAKE_BUILD_TYPE=Release + -DCMAKE_C_FLAGS=-Werror=missing-declarations + -DNIFTI_BUILD_APPLICATIONS=ON + -DUSE_CIFTI_CODE=ON + -DUSE_FSL_CODE=ON + -DFSLSTYLE=${{ matrix.fslstyle }} + -S ${{ github.workspace }} + - name: Build + run: cmake --build ${{ github.workspace }}/build + + windows: + # Twelve source files carry WIN32 or _MSC_VER guards and nothing has + # compiled them for as long as the workflows have existed, so a change + # that breaks the Windows path is invisible until a consumer reports + # it. zlib comes from vcpkg because the project requires it. + name: windows-latest msvc ${{ matrix.linkage }} + runs-on: windows-latest + strategy: + fail-fast: false + matrix: + include: + - linkage: static + shared: "OFF" + triplet: x64-windows-static + - linkage: shared + shared: "ON" + triplet: x64-windows + steps: + - uses: actions/checkout@v4 + - name: Install zlib and expat + run: vcpkg install zlib:${{ matrix.triplet }} expat:${{ matrix.triplet }} + - name: Configure CMake + shell: pwsh + run: | + $toolchain = Join-Path $env:VCPKG_INSTALLATION_ROOT 'scripts/buildsystems/vcpkg.cmake' + Write-Host "vcpkg toolchain: $toolchain" + if (-not (Test-Path $toolchain)) { throw "toolchain file not found: $toolchain" } + cmake -B "${{ github.workspace }}/build" ` + -DCMAKE_TOOLCHAIN_FILE="$toolchain" ` + -DVCPKG_TARGET_TRIPLET=${{ matrix.triplet }} ` + -DBUILD_SHARED_LIBS=${{ matrix.shared }} ` + -DCMAKE_RUNTIME_OUTPUT_DIRECTORY="${{ github.workspace }}/build/bin" ` + -DNIFTI_BUILD_APPLICATIONS=ON ` + -DUSE_NIFTI2_CODE=ON ` + -DUSE_CIFTI_CODE=ON ` + -S "${{ github.workspace }}" + - name: Build + run: cmake --build ${{ github.workspace }}/build --config Release + - name: Test + working-directory: ${{ github.workspace }}/build + # A test that cannot find a DLL can raise a modal dialog and block the + # runner rather than exit, so every test carries a timeout. + run: ctest --build-config Release --output-on-failure --no-tests=ignore --timeout 120 + workflow-triggers: + # A branch filter naming a branch that does not exist leaves the + # workflow configured but never scheduled, which is indistinguishable + # from one that runs and passes: no red check appears because no check + # appears at all. build.yml sat dead behind `branches: [ main ]` on a + # repository whose branch is master, and nothing detected it. + name: workflow triggers can match a branch + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install dependencies + run: sudo apt-get update && sudo apt-get install -y python3-yaml + - name: Check the branch filters + env: + DEFAULT_BRANCH: ${{ github.event.repository.default_branch }} + run: python3 .github/check_workflow_triggers.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 04b106ce..f6d230ae 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -34,6 +34,7 @@ project(NIFTI list(APPEND CMAKE_MODULE_PATH ${CMAKE_CURRENT_LIST_DIR}/cmake) include(nifti_macros) +include(nifti_warnings) set_property(GLOBAL PROPERTY nifti_installed_targets) @@ -163,7 +164,12 @@ if (NIFTI_BUILD_TESTING ) include(FetchContent) # fetch data a configure time to simplify tests # If new or changed data is needed, add that data to the https://github.com/NIFTI-Imaging/nifti-test-data repo # make a new release, and then update the URL and hash (shasum -a 256 ). - cmake_policy(SET CMP0169 OLD) + # CMP0169 (deprecating FetchContent_Populate) only exists from CMake 3.30; + # setting it unconditionally is a hard error on every earlier release, + # including the CMake 3.28 that Ubuntu 24.04 ships. + if(POLICY CMP0169) + cmake_policy(SET CMP0169 OLD) + endif() FetchContent_Declare( fetch_testing_data URL https://github.com/NIFTI-Imaging/nifti-test-data/archive/v3.0.2.tar.gz URL_HASH SHA256=5dafec078151018da7aaf3c941bd31f246f590bc34fa3fef29ce77a773db16a6 diff --git a/README.md b/README.md index 4dc5e21b..11dfb30e 100644 --- a/README.md +++ b/README.md @@ -85,10 +85,65 @@ packaging | spec file for building RPMs, and template package description for ## Instructions to build -command | description -------------|------------- -"make all" | results will be left in the directories: bin/ include/ lib/ -"make help" | will show more build options +The project builds with CMake: + +```sh +cmake -S . -B build -DCMAKE_BUILD_TYPE=Release +cmake --build build -j +ctest --test-dir build --output-on-failure +``` + +Useful options: `-DBUILD_SHARED_LIBS=ON`, `-DUSE_CIFTI_CODE=ON` (needs +expat), `-DUSE_FSL_CODE=ON`, and `-DDOWNLOAD_TEST_DATA=OFF` together with +`ctest -LE NEEDS_DATA` for a build with no network access. + +The top-level GNU `Makefile` still exists and still mostly works, but it +is not maintained and does not build the nifti2 or cifti libraries. + +## Checking the code + +The sanitizers are the quickest memory check: + +```sh +cmake -S . -B build-asan -DCMAKE_BUILD_TYPE=Debug \ + -DCMAKE_C_FLAGS="-fsanitize=address,undefined -fno-sanitize-recover=all" \ + -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" +cmake --build build-asan -j && ctest --test-dir build-asan --output-on-failure +``` + +For valgrind: + +```sh +ctest --test-dir build -T memcheck -E install_linking \ + --overwrite MemoryCheckCommandOptions="--trace-children=yes --leak-check=full" +``` + +`--trace-children=yes` matters: many of the tests are shell scripts that +exec the tools, so without it valgrind only ever inspects the shell and +reports a clean run. `install_linking` is excluded because it configures +and builds a whole CMake project, and tracing cmake and the compiler +through valgrind takes far longer than it is worth. + +`ctest -T memcheck` exits 0 even when valgrind reports defects, so read +`build/Testing/Temporary/MemoryChecker.*.log` rather than trusting the +exit status. + +Note that valgrind needs the C library's debug symbols and refuses to +start without them. Distributions that ship a stripped `ld.so` and no +debuginfo package for it -- Arch and its derivatives among them -- cannot +run it at all, and `DEBUGINFOD_URLS` does not help, because those builds +are not published to any debuginfod server. Run it in a container +instead: + +```sh +docker run --rm -v "$PWD":/src:ro -w /work ubuntu:24.04 bash -c ' + apt-get update && apt-get install -y build-essential cmake ninja-build \ + valgrind libc6-dbg zlib1g-dev libexpat1-dev + cmake -S /src -B /work/build -G Ninja -DCMAKE_BUILD_TYPE=Debug + cmake --build /work/build -j + ctest --test-dir /work/build -T memcheck -E install_linking \ + --overwrite MemoryCheckCommandOptions="--trace-children=yes --leak-check=full"' +``` ![NIFTI ICON](https://avatars0.githubusercontent.com/u/45666806?s=200&v=4) diff --git a/cifti/CMakeLists.txt b/cifti/CMakeLists.txt index d19b7524..88dcc657 100644 --- a/cifti/CMakeLists.txt +++ b/cifti/CMakeLists.txt @@ -26,3 +26,46 @@ if(NIFTI_BUILD_APPLICATIONS) install_nifti_target(${NIFTI_PACKAGE_PREFIX}afni_xml_tool) install_nifti_target(${NIFTI_PACKAGE_PREFIX}cifti_tool) endif() + +if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) + set(TEST_PREFIX "${NIFTI_PACKAGE_PREFIX}cifti") + # An unrecognized MatrixIndicesMap child is skipped, not dispatched on. + add_test( NAME ${TEST_PREFIX}_mim_summary_unknown_child + COMMAND $ + -as_cext -eval_cext -eval_type show_summary + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/mim_unknown_child.xml ) + # A recognized child is still displayed. + add_test( NAME ${TEST_PREFIX}_mim_summary_known_child + COMMAND $ + -as_cext -eval_cext -eval_type show_summary + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/mim_known_child.xml ) + set_tests_properties( ${TEST_PREFIX}_mim_summary_known_child + PROPERTIES PASS_REGULAR_EXPRESSION "BrainModel" ) + + # The CIFTI extension is found even when another extension precedes it. + add_test( NAME ${TEST_PREFIX}_tool_cext_not_first + COMMAND $ + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/cext_second_extension.nii + -disp_cext ) + set_tests_properties( ${TEST_PREFIX}_tool_cext_not_first + PROPERTIES PASS_REGULAR_EXPRESSION "demo" + FAIL_REGULAR_EXPRESSION "no CIFTI extension" ) + + # An extension payload that fills esize-8 with no NUL must not be read + # past its end; the regex pins that the payload is still parsed. + add_test( NAME ${TEST_PREFIX}_tool_unterminated_cext + COMMAND $ + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/cext_unterminated.nii + -eval_cext -eval_type show_summary ) + set_tests_properties( ${TEST_PREFIX}_tool_unterminated_cext + PROPERTIES PASS_REGULAR_EXPRESSION "MapName : demo" ) + + # Nesting past AXML_MAX_DEPTH must stay inside the skip block rather than + # walk off the fixed stack; the regex pins that the rest still parses. + add_test( NAME ${TEST_PREFIX}_deep_nesting + COMMAND $ + -as_cext -eval_cext -eval_type show_summary + -input ${CMAKE_CURRENT_LIST_DIR}/testdata/deep_nesting.xml ) + set_tests_properties( ${TEST_PREFIX}_deep_nesting + PROPERTIES PASS_REGULAR_EXPRESSION "BrainModel" ) +endif() diff --git a/cifti/afni_xml.c b/cifti/afni_xml.c index a9910a1b..35a7f64a 100644 --- a/cifti/afni_xml.c +++ b/cifti/afni_xml.c @@ -196,7 +196,7 @@ afni_xml_list axml_read_file(const char * fname, int read_data) { if( reset_xml_buf(xd, &buf, &bsize) ) break; - blen = fread(buf, 1, bsize, fp); + blen = fread(buf, 1, (size_t)bsize, fp); /* check for early termination */ bshort = loc_strnlen(buf, blen); @@ -204,14 +204,14 @@ afni_xml_list axml_read_file(const char * fname, int read_data) if( xd->verb > 1 ) fprintf(stderr,"-- AXML: truncating fbuffer from %u to %" PRId64 "\n", blen, bshort); - blen = (int)bshort; + blen = (unsigned)bshort; } done = blen < (unsigned) bsize; if(xd->verb > 4) fprintf(stderr,"-- XML_Parse # %d\n", pcount); pcount++; - if( XML_Parse(parser, buf, blen, done) == XML_STATUS_ERROR) { + if( XML_Parse(parser, buf, (int)blen, done) == XML_STATUS_ERROR) { fprintf(stderr,"** %s at line %u\n", XML_ErrorString(XML_GetErrorCode(parser)), (unsigned int)XML_GetCurrentLineNumber(parser)); @@ -284,7 +284,7 @@ afni_xml_list axml_read_buf(const char * buf_in, int64_t bin_len) /*--- replace fread with buffer copy ---*/ /* decide how much to copy and copy */ - if( bin_remain >= bsize ) blen = bsize; + if( bin_remain >= bsize ) blen = (unsigned)bsize; else blen = bin_remain; if(blen > 0 && blen <= (unsigned)bsize) { @@ -299,7 +299,7 @@ afni_xml_list axml_read_buf(const char * buf_in, int64_t bin_len) if(xd->verb > 4) fprintf(stderr,"-- XML_Parse # %d\n", pcount); pcount++; - if( XML_Parse(parser, buf, blen, done) == XML_STATUS_ERROR) { + if( XML_Parse(parser, buf, (int)blen, done) == XML_STATUS_ERROR) { fprintf(stderr,"** %s at line %u\n", XML_ErrorString(XML_GetErrorCode(parser)), (unsigned int)XML_GetCurrentLineNumber(parser)); @@ -412,7 +412,14 @@ afni_xml_t * new_afni_xml(const char * name) newp->xparent = NULL; newp->xchild = NULL; - if( name ) newp->name = strdup(name); + if( name ) { + newp->name = strdup(name); + if( ! newp->name ) { + fprintf(stderr,"** new_afni_xml: failed to copy name '%s'\n", name); + free(newp); + return NULL; + } + } return newp; } @@ -440,8 +447,8 @@ int axml_add_attrs(afni_xml_t * ax, const char ** attr) return 0; } - ax->attrs.name = (char **)malloc(natr*sizeof(char *)); - ax->attrs.value = (char **)malloc(natr*sizeof(char *)); + ax->attrs.name = (char **)malloc((size_t)natr * sizeof(char *)); + ax->attrs.value = (char **)malloc((size_t)natr * sizeof(char *)); /* failure? */ if( ! ax->attrs.name || ! ax->attrs.value ) { @@ -457,6 +464,11 @@ int axml_add_attrs(afni_xml_t * ax, const char ** attr) for(c = 0, aind = 0; attr[c]; c += 2, aind++) { ax->attrs.name[aind] = strdup(strip_whitespace(attr[c],0)); ax->attrs.value[aind] = strdup(strip_whitespace(attr[c+1],0)); + if( ! ax->attrs.name[aind] || ! ax->attrs.value[aind] ) { + fprintf(stderr,"** NAX: failed to copy attribute %d\n", aind); + ax->attrs.length = aind+1; /* so the partial pair is still freed */ + return 1; + } } return 0; @@ -654,7 +666,7 @@ static int reset_xml_buf(afni_xml_control * xd, char ** buf, int * bsize) fprintf(stderr,"++ update buf, %d to %d bytes\n",*bsize,xd->buf_size); *bsize = xd->buf_size; - *buf = (char *)safe_realloc(*buf, (*bsize+1) * sizeof(char)); + *buf = (char *)safe_realloc(*buf, (size_t)(*bsize+1) * sizeof(char)); if( ! *buf ) { fprintf(stderr,"** failed to alloc %d bytes of xml buf!\n", *bsize); *bsize = 0; @@ -722,8 +734,9 @@ static int epush(afni_xml_control * xd, const char * ename, const char ** attr) if( xd->verb > 3 ) show_attrs(xd, attr, 1); } - /* determine whether we should go into a skip block */ - if( errs ) xd->dskip = xd->depth; + /* determine whether we should go into a skip block; keep the outermost + such depth, since that is the one whose pop ends the skip */ + if( errs && ! xd->dskip ) xd->dskip = xd->depth; /* if we are in a skip block, do nothing but monitor stack */ if( xd->dskip ) { @@ -755,16 +768,16 @@ static int epop(afni_xml_control * xd, const char * ename) if( xd->wkeep ) xd->wkeep = 0; /* clear storage continuation */ if( xd->dskip ) { - if( xd->dskip == xd->depth ) xd->dskip = 0; /* clear */ - if( xd->verb > 3 ) fprintf(stderr,"-- skip=%d, depth=%d, skipping pop element '%s'\n", xd->dskip, xd->depth, ename); + + /* clear only after the element has been skipped, so that the stack + is not touched at a depth that was never pushed onto it */ + if( xd->dskip == xd->depth ) xd->dskip = 0; } else { process_popped_element(xd, ename); - } - if( ! xd->dskip ) { xd->stack[xd->depth-1] = NULL; /* should be irrelevant */ if( xd->verb > 4 ) { @@ -781,7 +794,21 @@ static int epop(afni_xml_control * xd, const char * ename) static int process_popped_element(afni_xml_control * xd, const char * ename) { afni_xml_t * ax; + + /* a stack slot is filled by the matching epush(). An element that was + skipped never fills one, so do not assume this slot holds a struct + with a name. */ + if( xd->depth <= 0 || xd->depth > AXML_MAX_DEPTH ) { + if( gAXD.verb ) fprintf(stderr,"** pop at depth %d!\n", xd->depth); + return 1; + } + ax = xd->stack[xd->depth-1]; + if( ! ax || ! ax->name ) { + if( gAXD.verb ) fprintf(stderr,"** pop of unfilled element '%s'!\n", + ename ? ename : "NULL"); + return 1; + } if( strcmp(ename, ax->name) ) { if( gAXD.verb ) fprintf(stderr,"** pop mismatch!\n"); @@ -799,7 +826,7 @@ static int add_to_xroot_list(afni_xml_control * xd, afni_xml_t * newp) xd->xroot->len++; xd->xroot->xlist = (afni_xml_t **)safe_realloc(xd->xroot->xlist, - xd->xroot->len * sizeof(afni_xml_t *)); + (size_t)xd->xroot->len * sizeof(afni_xml_t *)); if( ! xd->xroot->xlist ) { fprintf(stderr,"** failed to alloc %d AXMLT pointers\n", xd->xroot->len); return 1; @@ -817,7 +844,7 @@ static int add_to_xchild_list(afni_xml_t * parent, afni_xml_t * child) parent->nchild++; parent->xchild = (afni_xml_t **)safe_realloc(parent->xchild, - parent->nchild * sizeof(afni_xml_t *)); + (size_t)parent->nchild * sizeof(afni_xml_t *)); if( ! parent->xchild ) { fprintf(stderr,"** failed to alloc %d AXML pointers\n", parent->nchild); return 1; @@ -838,7 +865,11 @@ static afni_xml_t * make_afni_xml(const char * ename, const char ** attr) newp = new_afni_xml(ename); if( ! newp ) return NULL; - axml_add_attrs(newp, attr); + /* a failure here is an allocation failure; epush() skips on NULL */ + if( axml_add_attrs(newp, attr) ) { + axml_free_xml_t(newp); + return NULL; + } return newp; } @@ -875,7 +906,7 @@ static char * strip_whitespace(const char * str, int slen) /* make sure we have local space */ if( len > blen ) { /* allocate a bigger buffer */ - buf = (char *)safe_realloc(buf, (len+1) * sizeof(char)); + buf = (char *)safe_realloc(buf, (size_t)(len+1) * sizeof(char)); if( !buf ) { fprintf(stderr,"** failed to alloc wspace buf of len %d\n", len+1); return (char *)str; @@ -888,7 +919,7 @@ static char * strip_whitespace(const char * str, int slen) if( ifirst == len ) *buf = '\0'; else { - strncpy(buf, str+ifirst, len-ifirst-ilast); + strncpy(buf, str+ifirst, (size_t)(len-ifirst-ilast)); buf[len-ifirst-ilast] = '\0'; } @@ -940,14 +971,14 @@ static int append_to_string(char ** ostr, int * olen, newlen = *olen + ilen; - *ostr = (char *)safe_realloc(*ostr, newlen * sizeof(char)); + *ostr = (char *)safe_realloc(*ostr, (size_t)newlen * sizeof(char)); if( !*ostr ) { fprintf(stderr,"** AX.A2S: failed to alloc %d chars\n", newlen); return 1; } /* copy, starting at old nul char (if any), and terminate */ - strncpy((*ostr)+*olen-1, istr, ilen); + strncpy((*ostr)+*olen-1, istr, (size_t)ilen); (*ostr)[newlen-1] = '\0'; *olen = newlen; @@ -1021,7 +1052,8 @@ static int64_t loc_strnlen(const char * str, int64_t maxlen) const char * sptr; int64_t len; - for( sptr=str, len=0; *sptr && len= 4) || defined(__clang__) + #define CIF_API __attribute__ ((visibility ("default"))) + #else + #define CIF_API + #endif +#endif + /* --------------------------- prototypes --------------------------------- */ /* main interface */ -afni_xml_list axml_read_buf (const char * buf_in, int64_t bin_len); -afni_xml_list axml_read_file(const char * fname, int read_data); +CIF_API afni_xml_list axml_read_buf (const char * buf_in, int64_t bin_len); +CIF_API afni_xml_list axml_read_file(const char * fname, int read_data); -int axml_disp_xlist( const char *mesg, afni_xml_list * axlist, int verb); -int axml_disp_xml_t( const char *mesg, afni_xml_t * ax, int indent, int verb); +CIF_API int axml_disp_xlist( const char *mesg, afni_xml_list * axlist, int verb); +CIF_API int axml_disp_xml_t( const char *mesg, afni_xml_t * ax, int indent, int verb); /* create/free */ -afni_xml_t * new_afni_xml (const char * name); -int axml_add_attrs (afni_xml_t * ax, const char ** attr); -int axml_free_xml_t(afni_xml_t * ax); -int axml_free_xlist(afni_xml_list * axlist); +CIF_API afni_xml_t * new_afni_xml (const char * name); +CIF_API int axml_add_attrs (afni_xml_t * ax, const char ** attr); +CIF_API int axml_free_xml_t(afni_xml_t * ax); +CIF_API int axml_free_xlist(afni_xml_list * axlist); -char * axml_attr_value(afni_xml_t * ax, const char * name); -int axml_recur(int(*func)(FILE*,afni_xml_t*,int), afni_xml_t * ax); +CIF_API char * axml_attr_value(afni_xml_t * ax, const char * name); +CIF_API int axml_recur(int(*func)(FILE*,afni_xml_t*,int), afni_xml_t * ax); +CIF_API afni_xml_t * axml_recur_find_xml(int (*func)(afni_xml_t *, int), afni_xml_t * ax, + int depth, int max_depth); /* control API */ -int axml_set_verb ( int val ); -int axml_get_verb ( void ); -int axml_set_dstore ( int val ); -int axml_get_dstore ( void ); -int axml_set_indent ( int val ); -int axml_get_indent ( void ); -int axml_set_buf_size ( int val ); -int axml_get_buf_size ( void ); -int axml_set_wstream ( FILE *fp ); -FILE * axml_get_wstream ( void ); +CIF_API int axml_set_verb ( int val ); +CIF_API int axml_get_verb ( void ); +CIF_API int axml_set_dstore ( int val ); +CIF_API int axml_get_dstore ( void ); +CIF_API int axml_set_indent ( int val ); +CIF_API int axml_get_indent ( void ); +CIF_API int axml_set_buf_size ( int val ); +CIF_API int axml_get_buf_size ( void ); +CIF_API int axml_set_wstream ( FILE *fp ); +CIF_API FILE * axml_get_wstream ( void ); #endif /* AFNI_XML_H */ diff --git a/cifti/afni_xml_io.c b/cifti/afni_xml_io.c index a59f5c3e..ce60483b 100644 --- a/cifti/afni_xml_io.c +++ b/cifti/afni_xml_io.c @@ -111,7 +111,7 @@ int axio_text_to_binary(afni_xml_t * ax) int axio_num_tokens(const char * str, int64_t maxlen) { - char * sp = (char *)str; + const char * sp = str; int64_t ind, len, ntok; int intok; /* flag: are we inside a token? */ @@ -119,11 +119,11 @@ int axio_num_tokens(const char * str, int64_t maxlen) if( ! str || ! * str ) return 0; if( maxlen > 0 ) len = maxlen; - else len = strlen(str); + else len = (int64_t)strlen(str); ntok = 0; intok = 0; - for( ind = 0, sp = (char *)str; ind < len; ind++, sp++ ) { + for( ind = 0, sp = str; ind < len; ind++, sp++ ) { /* just look for state switches */ if( intok ) { if( isspace(*sp) || (*sp == ',') ) @@ -263,7 +263,8 @@ int axio_show_mim_summary(FILE * fp, const char * mesg, afni_xml_t * ax, int ver for( kid=0; kidnchild; kid++ ) { mind = get_map_index(xt->xchild[kid]); - if( kid >= 0 ) MIM_disp_funcs[mind](ofp, xt->xchild[kid], verb); + /* get_map_index() returns -1 for an unrecognized element name */ + if( mind >= 0 ) MIM_disp_funcs[mind](ofp, xt->xchild[kid], verb); } } @@ -472,7 +473,7 @@ static int dalloc_as_nifti_type(FILE * fp, afni_xml_t * ax, int64_t nvals, /* note number of bytes per value and number of values to allocate */ nifti_datatype_sizes(ax->btype, &nbyper, NULL); - ax->bdata = malloc(nbyper * ntok); + ax->bdata = malloc((size_t)(nbyper * ntok)); if( ! ax->bdata ) { fprintf(fp, "** axio_alloc: failed to allocate %" PRId64 " vals of size %d\n", ntok, nbyper); @@ -518,7 +519,8 @@ static int can_process_dtype(int dtype) * varies, unfortunately */ static int64_t text_to_i64(int64_t * result, const char * text, int64_t nvals) { - char * eptr, * sptr; + char * eptr; + const char * sptr; int64_t * rptr, val; int64_t nread; @@ -526,7 +528,7 @@ static int64_t text_to_i64(int64_t * result, const char * text, int64_t nvals) *result = 0; /* Initialize to zero in case of failure */ if( nvals <= 0 ) return 0; - sptr = (char *)text; + sptr = text; nread = 0; rptr = result; @@ -547,14 +549,15 @@ static int64_t text_to_i64(int64_t * result, const char * text, int64_t nvals) static int64_t text_to_f64(double * result, const char * text, int64_t nvals) { - char * eptr, * sptr; + char * eptr; + const char * sptr; double * rptr, val; int64_t nread; if( ! text || ! result) return 1; if( nvals <= 0 ) return 0; - sptr = (char *)text; + sptr = text; nread = 0; rptr = result; diff --git a/cifti/afni_xml_io.h b/cifti/afni_xml_io.h index ae8aeb2a..ee9b6a1b 100644 --- a/cifti/afni_xml_io.h +++ b/cifti/afni_xml_io.h @@ -50,29 +50,6 @@ - convert as in SUMA_Create_Fake_CIFTI() * ----------------------------------------------------------------------*/ -#ifndef CIF_API - #if defined(_WIN32) || defined(__CYGWIN__) - #if defined(CIFTI_BUILD_SHARED) - #ifdef __GNUC__ - #define CIF_API __attribute__ ((dllexport)) - #else - #define CIF_API __declspec( dllexport ) - #endif - #elif defined(CIFTI_USE_SHARED) - #ifdef __GNUC__ - #define CIF_API __attribute__ ((dllimport)) - #else - #define CIF_API __declspec( dllimport ) - #endif - #else - #define CIF_API - #endif - #elif (defined(__GNUC__) && __GNUC__ >= 4) || defined(__clang__) - #define CIF_API __attribute__ ((visibility ("default"))) - #else - #define CIF_API - #endif -#endif /* --------------------------- structures --------------------------------- */ diff --git a/cifti/afni_xml_tool.c b/cifti/afni_xml_tool.c index a1471ebf..0ed277bc 100644 --- a/cifti/afni_xml_tool.c +++ b/cifti/afni_xml_tool.c @@ -19,7 +19,7 @@ typedef struct { int xverb; } opts_t; -opts_t gopt; +static opts_t gopt; /* ----------------------------------------------------------------- */ diff --git a/cifti/cifti_tool.c b/cifti/cifti_tool.c index 488b70fb..718b610f 100644 --- a/cifti/cifti_tool.c +++ b/cifti/cifti_tool.c @@ -53,7 +53,7 @@ typedef struct { } opts_t; -opts_t gopt; +static opts_t gopt; /* ----------------------------------------------------------------- */ @@ -193,6 +193,9 @@ int process(opts_t * opts) if( opts->disp_cext ) disp_cifti_extension(nim, opts); if( opts->eval_cext ) eval_cifti_extension(ax, opts); + axml_free_xml_t(ax); + nifti_image_free(nim); + return 0; } @@ -207,19 +210,22 @@ int disp_cifti_extension(nifti_image * nim, opts_t * opts) opts->fout ? opts->fout : "DEFAULT" ); if( !nim ) return 1; - ext = nim->ext_list; + /* find the CIFTI extension, if any; it need not be first in the list */ + ext = NULL; for( ind = 0; ind < nim->num_ext; ind++ ) - if( ext->ecode == NIFTI_ECODE_CIFTI ) break; + if( nim->ext_list[ind].ecode == NIFTI_ECODE_CIFTI ) { + ext = nim->ext_list + ind; + break; + } fp = open_write_stream(opts->fout); - if( ext && ext->ecode != NIFTI_ECODE_CIFTI ) { + if( !ext ) { fprintf(fp, "** no CIFTI extension in %s\n",nim->fname?nim->fname:"NULL"); + close_stream(fp); return 1; } - if(ext) { - fprintf(fp, "%.*s\n", ext->esize-8, ext->edata); - } + fprintf(fp, "%.*s\n", ext->esize-8, ext->edata); /* possibly close file */ close_stream(fp); diff --git a/cifti/testdata/cext_second_extension.nii b/cifti/testdata/cext_second_extension.nii new file mode 100644 index 00000000..f3eca967 Binary files /dev/null and b/cifti/testdata/cext_second_extension.nii differ diff --git a/cifti/testdata/cext_unterminated.nii b/cifti/testdata/cext_unterminated.nii new file mode 100644 index 00000000..43eb9b73 Binary files /dev/null and b/cifti/testdata/cext_unterminated.nii differ diff --git a/cifti/testdata/deep_nesting.xml b/cifti/testdata/deep_nesting.xml new file mode 100644 index 00000000..cf26ee24 --- /dev/null +++ b/cifti/testdata/deep_nesting.xml @@ -0,0 +1,8 @@ + + + + + + + + diff --git a/cifti/testdata/mim_known_child.xml b/cifti/testdata/mim_known_child.xml new file mode 100644 index 00000000..30eecbdf --- /dev/null +++ b/cifti/testdata/mim_known_child.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/cifti/testdata/mim_unknown_child.xml b/cifti/testdata/mim_unknown_child.xml new file mode 100644 index 00000000..83c64772 --- /dev/null +++ b/cifti/testdata/mim_unknown_child.xml @@ -0,0 +1,7 @@ + + + + + + + diff --git a/cmake/collect_exported_symbols.sh b/cmake/collect_exported_symbols.sh new file mode 100755 index 00000000..2cdae6c0 --- /dev/null +++ b/cmake/collect_exported_symbols.sh @@ -0,0 +1,60 @@ +#!/bin/sh +# Collect the dynamic symbols exported by the built shared libraries. +# +# collect_exported_symbols.sh +# +# The output is sorted " " lines, so a diff against the +# committed baseline names both the library and the symbol that moved. +# +# The library name is reported with any version suffix stripped, so a +# SOVERSION bump does not rewrite every line of the baseline: +# libniftiio.so.3.0.0 is reported as libniftiio.so. +# +# Regenerate the baseline after an intended ABI change: +# cmake -B build -DBUILD_SHARED_LIBS=ON -DUSE_CIFTI_CODE=ON -DUSE_FSL_CODE=ON +# cmake --build build +# sh cmake/collect_exported_symbols.sh build cmake/exported_symbols_linux.txt + +set -eu + +if [ $# -ne 2 ]; then + echo "usage: $0 " >&2 + exit 2 +fi + +build_dir=$1 +output=$2 + +if [ ! -d "$build_dir" ]; then + echo "$0: no such build directory: $build_dir" >&2 + exit 1 +fi + +# Match versioned sonames too (libfoo.so.3.0.0), and resolve symlinks so +# libfoo.so -> libfoo.so.3.0.0 is not counted twice. +libs=$(find "$build_dir" -name 'lib*.so*' -exec readlink -f {} \; \ + | LC_ALL=C sort -u) + +if [ -z "$libs" ]; then + echo "$0: no shared libraries under $build_dir;" \ + "configure with -DBUILD_SHARED_LIBS=ON" >&2 + exit 1 +fi + +: > "$output.tmp" + +for lib in $libs; do + # libniftiio.so.3.0.0 -> libniftiio.so + name=$(basename "$lib" | sed -E 's/\.so(\.[0-9]+)*$/.so/') + # -D dynamic symbols, --defined-only drops imports; field 3 is the name. + nm -D --defined-only "$lib" \ + | awk 'NF >= 3 { print $3 }' \ + | grep -v '^$' \ + | sed "s|^|$name |" >> "$output.tmp" +done + +LC_ALL=C sort -u "$output.tmp" > "$output" +rm -f "$output.tmp" + +echo "$0: wrote $(wc -l < "$output") symbols from" \ + "$(awk '{print $1}' "$output" | LC_ALL=C sort -u | wc -l) libraries to $output" diff --git a/cmake/exported_symbols_linux.txt b/cmake/exported_symbols_linux.txt new file mode 100644 index 00000000..042959f5 --- /dev/null +++ b/cmake/exported_symbols_linux.txt @@ -0,0 +1,439 @@ +libcifti.so axio_cifti_from_ext +libcifti.so axio_find_map_name +libcifti.so axio_num_tokens +libcifti.so axio_read_buf +libcifti.so axio_read_cifti_file +libcifti.so axio_read_file +libcifti.so axio_show_attrs +libcifti.so axio_show_cifti_summary +libcifti.so axio_show_mim_summary +libcifti.so axio_text_to_binary +libcifti.so axml_add_attrs +libcifti.so axml_attr_value +libcifti.so axml_disp_xlist +libcifti.so axml_disp_xml_t +libcifti.so axml_free_xlist +libcifti.so axml_free_xml_t +libcifti.so axml_get_buf_size +libcifti.so axml_get_dstore +libcifti.so axml_get_indent +libcifti.so axml_get_verb +libcifti.so axml_get_wstream +libcifti.so axml_read_buf +libcifti.so axml_read_file +libcifti.so axml_recur +libcifti.so axml_recur_find_xml +libcifti.so axml_set_buf_size +libcifti.so axml_set_dstore +libcifti.so axml_set_indent +libcifti.so axml_set_verb +libcifti.so axml_set_wstream +libcifti.so new_afni_xml +libfslio.so AvwSwapHeader +libfslio.so FslBaseFileType +libfslio.so FslCheckForMultipleFileNames +libfslio.so FslCloneHeader +libfslio.so FslClose +libfslio.so FslFileExists +libfslio.so FslFileTypeString +libfslio.so FslGetAnalyzeOrigin +libfslio.so FslGetAuxFile +libfslio.so FslGetBufferAsScaledDouble +libfslio.so FslGetCalMinMax +libfslio.so FslGetDataType +libfslio.so FslGetDim +libfslio.so FslGetDimensionality +libfslio.so FslGetEnvOutputType +libfslio.so FslGetFileType +libfslio.so FslGetHdrImgNames +libfslio.so FslGetIgnoreMFQ +libfslio.so FslGetIntensityScaling +libfslio.so FslGetIntent +libfslio.so FslGetLeftRightOrder +libfslio.so FslGetMMCoord +libfslio.so FslGetOverrideOutputType +libfslio.so FslGetRigidXform +libfslio.so FslGetStdXform +libfslio.so FslGetTimeUnits +libfslio.so FslGetVolSize +libfslio.so FslGetVolumeAsScaledDouble +libfslio.so FslGetVoxCoord +libfslio.so FslGetVoxDim +libfslio.so FslGetWriteMode +libfslio.so FslInit +libfslio.so FslInitHeader +libfslio.so FslIsCompressedFileType +libfslio.so FslIsSingleFileType +libfslio.so FslMakeBaseName +libfslio.so FslOpen +libfslio.so FslReadAllVolumes +libfslio.so FslReadHeader +libfslio.so FslReadRawHeader +libfslio.so FslReadRowSeries +libfslio.so FslReadSliceSeries +libfslio.so FslReadTimeSeries +libfslio.so FslReadVolumes +libfslio.so FslSeekVolume +libfslio.so FslSetAnalyzeSform +libfslio.so FslSetAuxFile +libfslio.so FslSetCalMinMax +libfslio.so FslSetDataType +libfslio.so FslSetDim +libfslio.so FslSetDimensionality +libfslio.so FslSetFileType +libfslio.so FslSetIgnoreMFQ +libfslio.so FslSetInit +libfslio.so FslSetIntensityScaling +libfslio.so FslSetIntent +libfslio.so FslSetOverrideOutputType +libfslio.so FslSetRigidXform +libfslio.so FslSetStdXform +libfslio.so FslSetTimeUnits +libfslio.so FslSetVoxDim +libfslio.so FslSetWriteMode +libfslio.so FslWriteAllVolumes +libfslio.so FslWriteHeader +libfslio.so FslWriteVolumes +libfslio.so FslXOpen +libfslio.so convertBufferToScaledDouble +libfslio.so d3matrix +libfslio.so d4matrix +libfslio.so mat44_to_mat33 +libnifti2.so disp_nifti_1_header +libnifti2.so disp_nifti_2_header +libnifti2.so is_nifti_file +libnifti2.so is_valid_nifti_type +libnifti2.so nifti1_magic +libnifti2.so nifti2_magic +libnifti2.so nifti_add_extension +libnifti2.so nifti_alter_cifti_dims +libnifti2.so nifti_compiled_with_zlib +libnifti2.so nifti_convert_n1hdr2nim +libnifti2.so nifti_convert_n2hdr2nim +libnifti2.so nifti_convert_nim2n1hdr +libnifti2.so nifti_convert_nim2n2hdr +libnifti2.so nifti_copy_extensions +libnifti2.so nifti_copy_nim_info +libnifti2.so nifti_datatype_from_string +libnifti2.so nifti_datatype_is_valid +libnifti2.so nifti_datatype_sizes +libnifti2.so nifti_datatype_string +libnifti2.so nifti_datatype_to_string +libnifti2.so nifti_disp_lib_hist +libnifti2.so nifti_disp_lib_version +libnifti2.so nifti_disp_matrix_orient +libnifti2.so nifti_disp_type_list +libnifti2.so nifti_dmat33_colnorm +libnifti2.so nifti_dmat33_determ +libnifti2.so nifti_dmat33_inverse +libnifti2.so nifti_dmat33_mul +libnifti2.so nifti_dmat33_polar +libnifti2.so nifti_dmat33_rownorm +libnifti2.so nifti_dmat44_inverse +libnifti2.so nifti_dmat44_mul +libnifti2.so nifti_dmat44_to_mat44 +libnifti2.so nifti_dmat44_to_orientation +libnifti2.so nifti_dmat44_to_quatern +libnifti2.so nifti_fileexists +libnifti2.so nifti_find_file_extension +libnifti2.so nifti_findhdrname +libnifti2.so nifti_findimgname +libnifti2.so nifti_free_NBL +libnifti2.so nifti_free_extensions +libnifti2.so nifti_get_alter_cifti +libnifti2.so nifti_get_filesize +libnifti2.so nifti_get_int64list +libnifti2.so nifti_get_intlist +libnifti2.so nifti_get_volsize +libnifti2.so nifti_hdr1_looks_good +libnifti2.so nifti_hdr2_looks_good +libnifti2.so nifti_header_version +libnifti2.so nifti_image_free +libnifti2.so nifti_image_from_ascii +libnifti2.so nifti_image_infodump +libnifti2.so nifti_image_load +libnifti2.so nifti_image_load_bricks +libnifti2.so nifti_image_open +libnifti2.so nifti_image_read +libnifti2.so nifti_image_read_bricks +libnifti2.so nifti_image_to_ascii +libnifti2.so nifti_image_unload +libnifti2.so nifti_image_write +libnifti2.so nifti_image_write_bricks +libnifti2.so nifti_image_write_bricks_status +libnifti2.so nifti_image_write_hdr_img +libnifti2.so nifti_image_write_hdr_img2 +libnifti2.so nifti_image_write_status +libnifti2.so nifti_intent_string +libnifti2.so nifti_is_complete_filename +libnifti2.so nifti_is_gzfile +libnifti2.so nifti_is_inttype +libnifti2.so nifti_is_valid_datatype +libnifti2.so nifti_is_valid_ecode +libnifti2.so nifti_looks_like_cifti +libnifti2.so nifti_make_new_n1_header +libnifti2.so nifti_make_new_n2_header +libnifti2.so nifti_make_new_nim +libnifti2.so nifti_make_orthog_dmat44 +libnifti2.so nifti_make_orthog_mat44 +libnifti2.so nifti_makebasename +libnifti2.so nifti_makehdrname +libnifti2.so nifti_makeimgname +libnifti2.so nifti_mat33_colnorm +libnifti2.so nifti_mat33_determ +libnifti2.so nifti_mat33_inverse +libnifti2.so nifti_mat33_mul +libnifti2.so nifti_mat33_polar +libnifti2.so nifti_mat33_rownorm +libnifti2.so nifti_mat44_inverse +libnifti2.so nifti_mat44_mul +libnifti2.so nifti_mat44_to_dmat44 +libnifti2.so nifti_mat44_to_orientation +libnifti2.so nifti_mat44_to_quatern +libnifti2.so nifti_nim_has_valid_dims +libnifti2.so nifti_nim_is_valid +libnifti2.so nifti_orientation_string +libnifti2.so nifti_quatern_to_dmat44 +libnifti2.so nifti_quatern_to_mat44 +libnifti2.so nifti_read_ascii_image +libnifti2.so nifti_read_buffer +libnifti2.so nifti_read_collapsed_image +libnifti2.so nifti_read_header +libnifti2.so nifti_read_n1_hdr +libnifti2.so nifti_read_n2_hdr +libnifti2.so nifti_read_subregion_image +libnifti2.so nifti_set_allow_upper_fext +libnifti2.so nifti_set_alter_cifti +libnifti2.so nifti_set_debug_level +libnifti2.so nifti_set_filenames +libnifti2.so nifti_set_fix_floats +libnifti2.so nifti_set_iname_offset +libnifti2.so nifti_set_skip_blank_ext +libnifti2.so nifti_set_type_from_names +libnifti2.so nifti_short_order +libnifti2.so nifti_simple_init_nim +libnifti2.so nifti_slice_string +libnifti2.so nifti_strdup +libnifti2.so nifti_swap_16bytes +libnifti2.so nifti_swap_2bytes +libnifti2.so nifti_swap_4bytes +libnifti2.so nifti_swap_8bytes +libnifti2.so nifti_swap_Nbytes +libnifti2.so nifti_swap_as_analyze +libnifti2.so nifti_swap_as_nifti1 +libnifti2.so nifti_swap_as_nifti2 +libnifti2.so nifti_test_datatype_sizes +libnifti2.so nifti_type_and_names_match +libnifti2.so nifti_units_string +libnifti2.so nifti_update_dims_from_array +libnifti2.so nifti_valid_header_size +libnifti2.so nifti_validfilename +libnifti2.so nifti_write_all_data +libnifti2.so nifti_write_ascii_image +libnifti2.so nifti_write_buffer +libnifti2.so nifti_xform_string +libnifti2.so old_swap_nifti_header +libnifti2.so swap_nifti_header +libnifti2.so valid_nifti_brick_list +libnifti2.so valid_nifti_extensions +libnifticdf.so E0000 +libnifticdf.so E0001 +libnifticdf.so Xgamm +libnifticdf.so algdiv +libnifticdf.so alngam +libnifticdf.so alnrel +libnifticdf.so apser +libnifticdf.so basym +libnifticdf.so bcorr +libnifticdf.so betaln +libnifticdf.so bfrac +libnifticdf.so bgrat +libnifticdf.so bpser +libnifticdf.so bratio +libnifticdf.so brcmp1 +libnifticdf.so brcomp +libnifticdf.so bup +libnifticdf.so cdfbet +libnifticdf.so cdfbin +libnifticdf.so cdfchi +libnifticdf.so cdfchn +libnifticdf.so cdff +libnifticdf.so cdffnc +libnifticdf.so cdfgam +libnifticdf.so cdfnbn +libnifticdf.so cdfnor +libnifticdf.so cdfpoi +libnifticdf.so cdft +libnifticdf.so cumbet +libnifticdf.so cumbin +libnifticdf.so cumchi +libnifticdf.so cumchn +libnifticdf.so cumf +libnifticdf.so cumfnc +libnifticdf.so cumgam +libnifticdf.so cumnbn +libnifticdf.so cumnor +libnifticdf.so cumpoi +libnifticdf.so cumt +libnifticdf.so dbetrm +libnifticdf.so devlpl +libnifticdf.so dexpm1 +libnifticdf.so dinvnr +libnifticdf.so dinvr +libnifticdf.so dlanor +libnifticdf.so dln1mx +libnifticdf.so dln1px +libnifticdf.so dlnbet +libnifticdf.so dlngam +libnifticdf.so dstinv +libnifticdf.so dstrem +libnifticdf.so dstzr +libnifticdf.so dt1 +libnifticdf.so dzror +libnifticdf.so erf1 +libnifticdf.so erfc1 +libnifticdf.so esum +libnifticdf.so exparg +libnifticdf.so fifdint +libnifticdf.so fifdmax1 +libnifticdf.so fifdmin1 +libnifticdf.so fifdsign +libnifticdf.so fifidint +libnifticdf.so fifmod +libnifticdf.so fpser +libnifticdf.so ftnstop +libnifticdf.so gam1 +libnifticdf.so gaminv +libnifticdf.so gamln +libnifticdf.so gamln1 +libnifticdf.so grat1 +libnifticdf.so gratio +libnifticdf.so gsumln +libnifticdf.so inam +libnifticdf.so ipmpar +libnifticdf.so nifti_cdf2stat +libnifticdf.so nifti_intent_code +libnifticdf.so nifti_rcdf2stat +libnifticdf.so nifti_stat2cdf +libnifticdf.so nifti_stat2hzscore +libnifticdf.so nifti_stat2rcdf +libnifticdf.so nifti_stat2zscore +libnifticdf.so psi +libnifticdf.so rcomp +libnifticdf.so rexp +libnifticdf.so rlog +libnifticdf.so rlog1 +libnifticdf.so spmpar +libnifticdf.so stvaln +libniftiio.so disp_nifti_1_header +libniftiio.so is_nifti_file +libniftiio.so is_valid_nifti_type +libniftiio.so nifti_add_extension +libniftiio.so nifti_compiled_with_zlib +libniftiio.so nifti_convert_nhdr2nim +libniftiio.so nifti_convert_nim2nhdr +libniftiio.so nifti_copy_extensions +libniftiio.so nifti_copy_nim_info +libniftiio.so nifti_datatype_from_string +libniftiio.so nifti_datatype_is_valid +libniftiio.so nifti_datatype_sizes +libniftiio.so nifti_datatype_string +libniftiio.so nifti_datatype_to_string +libniftiio.so nifti_disp_lib_hist +libniftiio.so nifti_disp_lib_version +libniftiio.so nifti_disp_matrix_orient +libniftiio.so nifti_disp_type_list +libniftiio.so nifti_fileexists +libniftiio.so nifti_find_file_extension +libniftiio.so nifti_findhdrname +libniftiio.so nifti_findimgname +libniftiio.so nifti_free_NBL +libniftiio.so nifti_free_extensions +libniftiio.so nifti_get_filesize +libniftiio.so nifti_get_intlist +libniftiio.so nifti_get_volsize +libniftiio.so nifti_hdr_looks_good +libniftiio.so nifti_image_free +libniftiio.so nifti_image_from_ascii +libniftiio.so nifti_image_infodump +libniftiio.so nifti_image_load +libniftiio.so nifti_image_load_bricks +libniftiio.so nifti_image_open +libniftiio.so nifti_image_read +libniftiio.so nifti_image_read_bricks +libniftiio.so nifti_image_to_ascii +libniftiio.so nifti_image_unload +libniftiio.so nifti_image_write +libniftiio.so nifti_image_write_bricks +libniftiio.so nifti_image_write_bricks_status +libniftiio.so nifti_image_write_hdr_img +libniftiio.so nifti_image_write_hdr_img2 +libniftiio.so nifti_image_write_status +libniftiio.so nifti_intent_string +libniftiio.so nifti_is_complete_filename +libniftiio.so nifti_is_gzfile +libniftiio.so nifti_is_inttype +libniftiio.so nifti_is_valid_datatype +libniftiio.so nifti_is_valid_ecode +libniftiio.so nifti_make_new_header +libniftiio.so nifti_make_new_nim +libniftiio.so nifti_make_orthog_mat44 +libniftiio.so nifti_makebasename +libniftiio.so nifti_makehdrname +libniftiio.so nifti_makeimgname +libniftiio.so nifti_mat33_colnorm +libniftiio.so nifti_mat33_determ +libniftiio.so nifti_mat33_inverse +libniftiio.so nifti_mat33_mul +libniftiio.so nifti_mat33_polar +libniftiio.so nifti_mat33_rownorm +libniftiio.so nifti_mat44_inverse +libniftiio.so nifti_mat44_to_orientation +libniftiio.so nifti_mat44_to_quatern +libniftiio.so nifti_nim_has_valid_dims +libniftiio.so nifti_nim_is_valid +libniftiio.so nifti_orientation_string +libniftiio.so nifti_quatern_to_mat44 +libniftiio.so nifti_read_ascii_image +libniftiio.so nifti_read_buffer +libniftiio.so nifti_read_collapsed_image +libniftiio.so nifti_read_header +libniftiio.so nifti_read_subregion_image +libniftiio.so nifti_set_allow_upper_fext +libniftiio.so nifti_set_debug_level +libniftiio.so nifti_set_filenames +libniftiio.so nifti_set_fix_floats +libniftiio.so nifti_set_iname_offset +libniftiio.so nifti_set_skip_blank_ext +libniftiio.so nifti_set_type_from_names +libniftiio.so nifti_short_order +libniftiio.so nifti_simple_init_nim +libniftiio.so nifti_slice_string +libniftiio.so nifti_strdup +libniftiio.so nifti_swap_16bytes +libniftiio.so nifti_swap_2bytes +libniftiio.so nifti_swap_4bytes +libniftiio.so nifti_swap_8bytes +libniftiio.so nifti_swap_Nbytes +libniftiio.so nifti_swap_as_analyze +libniftiio.so nifti_test_datatype_sizes +libniftiio.so nifti_type_and_names_match +libniftiio.so nifti_units_string +libniftiio.so nifti_update_dims_from_array +libniftiio.so nifti_validfilename +libniftiio.so nifti_write_all_data +libniftiio.so nifti_write_ascii_image +libniftiio.so nifti_write_buffer +libniftiio.so nifti_xform_string +libniftiio.so old_swap_nifti_header +libniftiio.so swap_nifti_header +libniftiio.so valid_nifti_brick_list +libniftiio.so valid_nifti_extensions +libznz.so Xznzclose +libznz.so znzopen +libznz.so znzputs +libznz.so znzread +libznz.so znzrewind +libznz.so znzseek +libznz.so znztell +libznz.so znzwrite diff --git a/cmake/travis_dashboard.cmake b/cmake/github_dashboard.cmake similarity index 90% rename from cmake/travis_dashboard.cmake rename to cmake/github_dashboard.cmake index 680e0031..8d358a70 100644 --- a/cmake/travis_dashboard.cmake +++ b/cmake/github_dashboard.cmake @@ -21,8 +21,10 @@ function(set_from_env var env_var) endif() endfunction() -set_from_env(CTEST_SITE "TRAVIS_APP_HOST" REQUIRED) -set(CTEST_SITE "travis.${CTEST_SITE}") +# RUNNER_OS is set by GitHub Actions ("Linux", "macOS", "Windows"). The +# default keeps the script usable from a developer machine. +set_from_env(CTEST_SITE "RUNNER_OS" DEFAULT "unknown") +set(CTEST_SITE "github.${CTEST_SITE}") set(CTEST_UPDATE_VERSION_ONLY 1) # https://gitlab.kitware.com/cmake/community/wikis/doc/ctest/Scripting-Of-CTest @@ -55,11 +57,11 @@ if(NOT CTEST_BUILD_NAME) set(branch "-$ENV{SYSTEM_PULLREQUEST_SOURCEBRANCH}") set(dashboard_git_branch "$ENV{SYSTEM_PULLREQUEST_SOURCEBRANCH}") set(dashboard_model "Experimental") - elseif(ENV{BUILD_SOURCEBRANCHNAME} STREQUAL "master") + elseif("$ENV{BUILD_SOURCEBRANCHNAME}" STREQUAL "master") set(branch "-master") set(dashboard_git_branch "$ENV{BUILD_SOURCEBRANCHNAME}") set(dashboard_model "Continuous") - elseif(ENV{BUILD_SOURCEBRANCHNAME} STREQUAL "nightly-master") + elseif("$ENV{BUILD_SOURCEBRANCHNAME}" STREQUAL "nightly-master") set(branch "-nightly-master") set(dashboard_git_branch "$ENV{BUILD_SOURCEBRANCHNAME}") set(dashboard_model "Nightly") @@ -76,7 +78,7 @@ if(NOT CTEST_BUILD_NAME) endif() set(CTEST_BUILD_NAME - "$ENV{BLDPREFIX}_$ENV{TRAVIS_OS_NAME}-$ENV{BUILD_BUILDID}_${pr}_${branch}") + "$ENV{BLDPREFIX}_$ENV{RUNNER_OS}-$ENV{BUILD_BUILDID}_${pr}_${branch}") endif() set(dashboard_cache " diff --git a/cmake/local_dashboard.cmake b/cmake/local_dashboard.cmake index a69588c1..c85a9dbd 100644 --- a/cmake/local_dashboard.cmake +++ b/cmake/local_dashboard.cmake @@ -32,9 +32,8 @@ function(set_from_env var env_var) endif() endfunction() -#set_from_env(CTEST_SITE "TRAVIS_APP_HOST" REQUIRED) cmake_host_system_information(RESULT CTEST_SITE QUERY HOSTNAME) -set(CTEST_SITE "travis.${CTEST_SITE}") +set(CTEST_SITE "local.${CTEST_SITE}") set(CTEST_UPDATE_VERSION_ONLY 1) set_from_env(PARALLEL_LEVEL "PARALLEL_LEVEL" DEFAULT 8) @@ -59,11 +58,11 @@ if(NOT CTEST_BUILD_NAME) set(branch "-$ENV{SYSTEM_PULLREQUEST_SOURCEBRANCH}") set(dashboard_git_branch "$ENV{SYSTEM_PULLREQUEST_SOURCEBRANCH}") set(dashboard_model "Experimental") - elseif(ENV{BUILD_SOURCEBRANCHNAME} STREQUAL "master") + elseif("$ENV{BUILD_SOURCEBRANCHNAME}" STREQUAL "master") set(branch "-master") set(dashboard_git_branch "$ENV{BUILD_SOURCEBRANCHNAME}") set(dashboard_model "Continuous") - elseif(ENV{BUILD_SOURCEBRANCHNAME} STREQUAL "nightly-master") + elseif("$ENV{BUILD_SOURCEBRANCHNAME}" STREQUAL "nightly-master") set(branch "-nightly-master") set(dashboard_git_branch "$ENV{BUILD_SOURCEBRANCHNAME}") set(dashboard_model "Nightly") @@ -80,7 +79,7 @@ if(NOT CTEST_BUILD_NAME) endif() set(CTEST_BUILD_NAME - "$ENV{TRAVIS_OS_NAME}-$ENV{BUILD_BUILDID}${pr}${branch}") + "$ENV{RUNNER_OS}-$ENV{BUILD_BUILDID}${pr}${branch}") endif() set(dashboard_cache " diff --git a/cmake/nifti_common.cmake b/cmake/nifti_common.cmake index e766f738..ed8f85b9 100644 --- a/cmake/nifti_common.cmake +++ b/cmake/nifti_common.cmake @@ -267,8 +267,10 @@ endif() set(CTEST_CHECKOUT_COMMAND "\"${CMAKE_COMMAND}\" -P \"${ctest_checkout_script}\"") # CTest delayed initialization is broken, so we put the # CTestConfig.cmake info here. - set(CTEST_NIGHTLY_START_TIME "01:00:00 UTC") - set(CTEST_DROP_METHOD "http") + # Keep these in agreement with CTestConfig.cmake; a nightly start time + # that disagrees files builds under the wrong day on the dashboard. + set(CTEST_NIGHTLY_START_TIME "00:00:00 EST") + set(CTEST_DROP_METHOD "https") set(CTEST_DROP_SITE "my.cdash.org") set(CTEST_DROP_LOCATION "/submit.php?project=nifti_clib") set(CTEST_DROP_SITE_CDASH TRUE) diff --git a/cmake/nifti_warnings.cmake b/cmake/nifti_warnings.cmake new file mode 100644 index 00000000..65553fe4 --- /dev/null +++ b/cmake/nifti_warnings.cmake @@ -0,0 +1,129 @@ +# +# Compiler warning flags shared by every nifti_clib target. +# +# The set below is the CLEAN SET: every flag here has been measured at +# zero warnings across the whole tree. A flag is added only once the +# tree is already clean under it, so that a warning always means a new +# defect rather than more noise. The FUTURE SET at the bottom of this +# file records the flags that are wanted but not yet earned, each with +# its measured hit count. +# +# Run the census before promoting anything out of the future set: +# +# cmake -G Ninja -S . -B build-census -DCMAKE_BUILD_TYPE=Release \ +# -DUSE_CIFTI_CODE=ON -DUSE_FSL_CODE=ON -DFSLSTYLE=ON \ +# -DCMAKE_C_FLAGS="" +# ninja -C build-census -k 0 2>&1 | tee census.log +# grep -oE '\[-W[a-z0-9-]+\]' census.log | sort | uniq -c | sort -rn +# +# Warnings are NOT errors by default; set NIFTI_WARNINGS_AS_ERRORS=ON to +# make CI fail on them. That option is only safe to turn on in CI once +# the future set below is empty. +# + +option(NIFTI_ENABLE_WARNINGS "Enable the project's compiler warning set" ON) +option(NIFTI_WARNINGS_AS_ERRORS "Treat compiler warnings as errors" OFF) +mark_as_advanced(NIFTI_ENABLE_WARNINGS NIFTI_WARNINGS_AS_ERRORS) + +if(NOT NIFTI_ENABLE_WARNINGS) + return() +endif() + +set(_nifti_warnings "") + +# --------------------------------------------------------------------- +# CLEAN SET - measured at zero, GCC and Clang +# --------------------------------------------------------------------- +if(CMAKE_C_COMPILER_ID MATCHES "GNU|Clang|AppleClang") + list(APPEND _nifti_warnings + -Wall + -Wpedantic + -Wformat=2 # printf/scanf checking, including nonliteral + -Wmissing-declarations + -Wnull-dereference + -Wpointer-arith # arithmetic on void * or function pointers + -Wredundant-decls + -Wshadow + -Wstrict-prototypes + -Wswitch-enum + -Wundef # #if on an undefined identifier + -Wvla # variable length arrays + -Wwrite-strings # string literals are const + ) +endif() + +# --------------------------------------------------------------------- +# CLEAN SET - Clang only +# --------------------------------------------------------------------- +if(CMAKE_C_COMPILER_ID MATCHES "Clang|AppleClang") + list(APPEND _nifti_warnings + -Wcomma + ) +endif() + +if(NIFTI_WARNINGS_AS_ERRORS) + if(MSVC) + list(APPEND _nifti_warnings /WX) + else() + list(APPEND _nifti_warnings -Werror) + endif() +endif() + +add_compile_options(${_nifti_warnings}) +unset(_nifti_warnings) + +# ===================================================================== +# FUTURE SET - wanted, not yet earned +# ===================================================================== +# +# Counts measured 2026-09-21 at d773c59, AppleClang 21.0.0, Release, +# USE_CIFTI_CODE=ON USE_FSL_CODE=ON FSLSTYLE=ON, NIFTI_BUILD_TESTING=OFF. +# Promote a flag to the clean set above only in a PR that follows the +# PR fixing its warnings, so CI is green at every step. +# +# flag hits fix +# ------------------------------------------------------------------ +# -Wsign-compare (via -Wextra) 3 PR #51 +# -Wcast-qual 9 PR #45 +# -Wmissing-prototypes 18 PR #37 +# -Wsign-conversion 200 PR #53 / #52, split by dir: +# znzlib 4, cifti 14, +# fsliolib 45, nifti2 58, +# niftilib 79 +# +# Clang-only: +# -Wnewline-eof 1 PR #35 covers one file only +# -Wmissing-variable-declarations 2 +# -Wconditional-uninitialized 11 relates to PR #47 / #48 +# -Wcast-align 19 relates to PR #44 +# -Wshorten-64-to-32 31 relates to PR #52 +# -Wextra-semi-stmt 74 relates to PR #49 +# +# -Wextra is held back only because it implies -Wsign-compare; once +# PR #51 lands it moves to the clean set with its 3 hits resolved. +# +# NOT MEASURED, do not add without a census first: +# +# GCC-only -Wcast-align=strict, -Wduplicated-branches, +# -Wduplicated-cond, -Wjump-misses-init, -Wlogical-op, +# -Wold-style-definition +# GNU 13.3.0 runs in CI (ubuntu-latest) but these flags +# have never been enabled, so their counts are unknown. +# +# MSVC /W3 +# There is no Windows job in any workflow, so nothing +# compiles this branch at all. Adding /W3 without a +# Windows CI job asserts a cleanliness nobody can check. +# +# DELIBERATELY EXCLUDED, with reasons: +# +# -Wdouble-promotion ~240 hits. Silencing them means calling sqrtf() +# instead of sqrt(), which changes the numerical +# results of the quaternion and matrix code. +# -Wfloat-equal ~150 hits. Most are deliberate tests against an +# exact 0.0 or a sentinel, which is correct here. +# -Wconversion Largely subsumed by -Wsign-conversion, and the +# remainder overlaps -Wdouble-promotion above. +# -Wunsafe-buffer-usage clang-only, aimed at C++ span/array types that +# do not exist in this codebase. +# diff --git a/fsliolib/fslio.c b/fsliolib/fslio.c index 5b775e77..44053fcf 100644 --- a/fsliolib/fslio.c +++ b/fsliolib/fslio.c @@ -28,6 +28,18 @@ #include "fslio.h" #include "assert.h" +/* Internal to this file. Nothing in the tree calls them across a + translation unit, no header declares them, and no public source + outside a vendored copy of this file references them. A downstream + project that needs one declares it in fslio.h with FSL_API. */ +static int FslIsValidFileType(int filetype); +static int FslGetFileType2(const FSLIO *fslio, int quiet); +static int FslFileType(const char *fname); +static int FslGetReadFileType(const FSLIO *fslio); +static void FslInit4Write(FSLIO* fslio, const char* filename, int ft); +static int fsl_fileexists(const char* fname); +static int check_for_multiple_filenames(const char* filename); + static int FslIgnoreMFQ=0; static int FslOverrideOutputType=-1; @@ -56,7 +68,7 @@ const char* FslFileTypeString(int filetype) } -int FslIsValidFileType(int filetype) +static int FslIsValidFileType(int filetype) { if ( (filetype!=FSL_TYPE_ANALYZE) && (filetype!=FSL_TYPE_ANALYZE_GZ) && (filetype!=FSL_TYPE_NIFTI) && (filetype!=FSL_TYPE_NIFTI_GZ) && @@ -85,9 +97,8 @@ int FslBaseFileType(int filetype) } -int FslGetFileType2(const FSLIO *fslio, int quiet) +static int FslGetFileType2(const FSLIO *fslio, int quiet) { - FSLIO *mutablefslio; if (fslio==NULL) FSLIOERR("FslGetFileType: Null pointer passed for FSLIO"); if ( (fslio->file_mode==FSL_TYPE_MINC) || (fslio->file_mode==FSL_TYPE_MINC_GZ) ) { return fslio->file_mode; @@ -100,8 +111,7 @@ int FslGetFileType2(const FSLIO *fslio, int quiet) fprintf(stderr,"Warning: nifti structure and fsl structure disagree on file type\n"); fprintf(stderr,"nifti = %d and fslio = %d\n",fslio->niftiptr->nifti_type,fslio->file_mode); } - mutablefslio = (FSLIO *) fslio; /* dodgy and will generate warnings */ - mutablefslio->niftiptr->nifti_type = FslBaseFileType(fslio->file_mode); + fslio->niftiptr->nifti_type = FslBaseFileType(fslio->file_mode); return fslio->file_mode; } } @@ -190,7 +200,7 @@ int FslGetEnvOutputType(void) } -int FslFileType(const char* fname) +static int FslFileType(const char* fname) { /* return type is FSL_TYPE_* or -1 to indicate undetermined */ /* use name as first priority but if that is ambiguous then resolve using environment */ @@ -224,7 +234,7 @@ int FslFileType(const char* fname) /************************************************************ * FslGetReadFileType ************************************************************/ -/*! \fn int FslGetReadFileType(const FSLIO *fslio) +/*! \fn static int FslGetReadFileType(const FSLIO *fslio) \brief return the best estimate of the true file type This function is used to return the best estimate of the true file type once @@ -389,7 +399,7 @@ void FslSetInit(FSLIO* fslio) -void FslInit4Write(FSLIO* fslio, const char* filename, int ft) +static void FslInit4Write(FSLIO* fslio, const char* filename, int ft) { /* ft determines filetype if ft>=0*/ int imgtype; @@ -502,7 +512,7 @@ void FslCloneHeader(FSLIO *dest, const FSLIO *src) } -int fsl_fileexists(const char* fname) +static int fsl_fileexists(const char* fname) { znzFile fp; fp = znzopen( fname , "rb" , 1 ) ; @@ -559,7 +569,7 @@ int FslCheckForMultipleFileNames(const char* filename) -int check_for_multiple_filenames(const char* filename) +static int check_for_multiple_filenames(const char* filename) { char *basename, *tmpname; char *otype; @@ -809,7 +819,6 @@ void* FslReadAllVolumes(FSLIO* fslio, char* filename) /* check for failure, from David Akers */ if (fslio->niftiptr == NULL) { FSLIOERR("FslReadAllVolumes: error reading NIfTI image"); - return(NULL); } FslSetFileType(fslio,fslio->niftiptr->nifti_type); @@ -873,7 +882,7 @@ void FslWriteAllVolumes(FSLIO *fslio, const void *buffer) FslGetDim(fslio,&x,&y,&z,&t); FslWriteHeader(fslio); - FslWriteVolumes(fslio,buffer,t); + FslWriteVolumes(fslio,buffer, (size_t)t); return; } @@ -915,13 +924,14 @@ size_t FslWriteVolumes(FSLIO *fslio, const void *buffer, size_t nvols) && (FslGetLeftRightOrder(fslio)==FSL_NEUROLOGICAL) ) { /* If it is Analyze and Neurological order then SWAP DATA into Radiological order */ /* This is nasty - but what else can be done?!? */ - char *tmpbuf, *inbuf; + char *tmpbuf; + const char *inbuf; long int x, b, n, nrows; short nx, ny, nz, nv; - inbuf = (char *) buffer; + inbuf = buffer; tmpbuf = (char *)calloc(nbytes,1); FslGetDim(fslio,&nx,&ny,&nz,&nv); - nrows = nbytes / (nx * bpv); + nrows = (long int)(nbytes / ((size_t)nx * (size_t)bpv)); for (n=0; n=z)) FSLIOERR("FslReadSliceSeries: slice outside valid range"); - slbytes = x * y * (FslGetDataType(fslio, &type) / 8); + slbytes = (size_t)x * (size_t)y * (FslGetDataType(fslio, &type) / 8); volbytes = slbytes * z; - orig_offset = znztell(fslio->fileptr); - znzseek(fslio->fileptr, slbytes*slice, SEEK_CUR); + orig_offset = (size_t)znztell(fslio->fileptr); + znzseek(fslio->fileptr, (znz_off_t)(slbytes*(size_t)slice), SEEK_CUR); for (n=0; n0) znzseek(fslio->fileptr, volbytes - slbytes, SEEK_CUR); + if (n>0) znzseek(fslio->fileptr, (znz_off_t)(volbytes - slbytes), SEEK_CUR); if (znzread((char *)buffer+n*slbytes, 1, slbytes, fslio->fileptr) != slbytes) FSLIOERR("FslReadSliceSeries: failed to read values"); if (fslio->niftiptr->byteorder != nifti_short_order()) - nifti_swap_Nbytes(slbytes / fslio->niftiptr->swapsize, + nifti_swap_Nbytes(slbytes / (size_t)fslio->niftiptr->swapsize, fslio->niftiptr->swapsize, (char *)buffer+n*slbytes); } /* restore file pointer to original position */ - znzseek(fslio->fileptr,orig_offset,SEEK_SET); + znzseek(fslio->fileptr,(znz_off_t)orig_offset,SEEK_SET); return n; } if (fslio->mincptr!=NULL) { @@ -1070,20 +1080,20 @@ size_t FslReadRowSeries(FSLIO *fslio, void *buffer, short row, short slice, size slbytes = rowbytes * y; volbytes = slbytes * z; - orig_offset = znztell(fslio->fileptr); - znzseek(fslio->fileptr, rowbytes*row + slbytes*slice, SEEK_CUR); + orig_offset = (size_t)znztell(fslio->fileptr); + znzseek(fslio->fileptr, (znz_off_t)(rowbytes*(size_t)row + slbytes*(size_t)slice), SEEK_CUR); for (n=0; n0) znzseek(fslio->fileptr, volbytes - rowbytes, SEEK_CUR); + if (n>0) znzseek(fslio->fileptr, (znz_off_t)(volbytes - rowbytes), SEEK_CUR); if (znzread((char *)buffer+n*rowbytes, 1, rowbytes, fslio->fileptr) != rowbytes) FSLIOERR("FslReadRowSeries: failed to read values"); if (fslio->niftiptr->byteorder != nifti_short_order()) - nifti_swap_Nbytes(rowbytes / fslio->niftiptr->swapsize, + nifti_swap_Nbytes(rowbytes / (size_t)fslio->niftiptr->swapsize, fslio->niftiptr->swapsize, (char *)buffer+n*rowbytes); } /* restore file pointer to original position */ - znzseek(fslio->fileptr,orig_offset,SEEK_SET); + znzseek(fslio->fileptr,(znz_off_t)orig_offset,SEEK_SET); return n; } if (fslio->mincptr!=NULL) { @@ -1129,14 +1139,14 @@ size_t FslReadTimeSeries(FSLIO *fslio, void *buffer, short xVox, short yVox, sho if ((zVox<0) || (zVox >=zdim)) FSLIOERR("FslReadTimeSeries: voxel outside valid range"); wordsize = fslio->niftiptr->nbyper; - volbytes = xdim * ydim * zdim * wordsize; + volbytes = (size_t)xdim * (size_t)ydim * (size_t)zdim * wordsize; - orig_offset = znztell(fslio->fileptr); + orig_offset = (size_t)znztell(fslio->fileptr); offset = ((ydim * zVox + yVox) * xdim + xVox) * wordsize; - znzseek(fslio->fileptr,offset,SEEK_CUR); + znzseek(fslio->fileptr,(znz_off_t)offset,SEEK_CUR); for (n=0; n0) znzseek(fslio->fileptr, volbytes - wordsize, SEEK_CUR); + if (n>0) znzseek(fslio->fileptr, (znz_off_t)(volbytes - wordsize), SEEK_CUR); if (znzread((char *)buffer+(n*wordsize), 1, wordsize,fslio->fileptr) != wordsize) FSLIOERR("FslReadTimeSeries: failed to read values"); if (fslio->niftiptr->byteorder != nifti_short_order()) @@ -1145,7 +1155,7 @@ size_t FslReadTimeSeries(FSLIO *fslio, void *buffer, short xVox, short yVox, sho } /* restore file pointer to original position */ - znzseek(fslio->fileptr,orig_offset,SEEK_SET); + znzseek(fslio->fileptr,(znz_off_t)orig_offset,SEEK_SET); return n; } @@ -1215,8 +1225,11 @@ void FslSetDim(FSLIO *fslio, short x, short y, short z, short v) fslio->niftiptr->dim[6] = fslio->niftiptr->nv; fslio->niftiptr->dim[7] = fslio->niftiptr->nw; - fslio->niftiptr->nvox = fslio->niftiptr->nx * fslio->niftiptr->ny * fslio->niftiptr->nz - * fslio->niftiptr->nt * fslio->niftiptr->nu * fslio->niftiptr->nv * fslio->niftiptr->nw ; + fslio->niftiptr->nvox = + (size_t)fslio->niftiptr->nx * (size_t)fslio->niftiptr->ny + * (size_t)fslio->niftiptr->nz * (size_t)fslio->niftiptr->nt + * (size_t)fslio->niftiptr->nu * (size_t)fslio->niftiptr->nv + * (size_t)fslio->niftiptr->nw ; } if (fslio->mincptr!=NULL) { @@ -1257,7 +1270,7 @@ void FslGetDimensionality(FSLIO *fslio, size_t *dim) { if (fslio==NULL) FSLIOERR("FslGetDimensionality: Null pointer passed for FSLIO"); if (fslio->niftiptr!=NULL) { - *dim = fslio->niftiptr->ndim; + *dim = (size_t)fslio->niftiptr->ndim; } if (fslio->mincptr!=NULL) { fprintf(stderr,"Warning:: Minc is not yet supported\n"); @@ -1342,8 +1355,9 @@ void FslGetAuxFile(FSLIO *fslio,char *aux_file) { if (fslio==NULL) FSLIOERR("FslGetAuxFile: Null pointer passed for FSLIO"); if (fslio->niftiptr!=NULL) { - strncpy(aux_file,fslio->niftiptr->aux_file, 24); - aux_file[24-1] = '\0'; + /* aux_file must have room for sizeof(nifti_1_header::aux_file) bytes. */ + strncpy(aux_file,fslio->niftiptr->aux_file,sizeof(fslio->niftiptr->aux_file)-1); + aux_file[sizeof(fslio->niftiptr->aux_file)-1] = '\0'; } if (fslio->mincptr!=NULL) { fprintf(stderr,"Warning:: Minc is not yet supported\n"); @@ -1355,8 +1369,8 @@ void FslSetAuxFile(FSLIO *fslio,const char *aux_file) { if (fslio==NULL) FSLIOERR("FslSetAuxFile: Null pointer passed for FSLIO"); if (fslio->niftiptr!=NULL) { - strncpy(fslio->niftiptr->aux_file, aux_file, 24); - fslio->niftiptr->aux_file[24-1] = '\0'; + strncpy(fslio->niftiptr->aux_file,aux_file,sizeof(fslio->niftiptr->aux_file)-1); + fslio->niftiptr->aux_file[sizeof(fslio->niftiptr->aux_file)-1] = '\0'; } if (fslio->mincptr!=NULL) { fprintf(stderr,"Warning:: Minc is not yet supported\n"); @@ -1364,7 +1378,9 @@ void FslSetAuxFile(FSLIO *fslio,const char *aux_file) } -void FslSetVoxUnits(FSLIO *fslio, const char *units) +#if 0 +/* No caller in this file, no header declares them, and no public source uses them. */ +static void FslSetVoxUnits(FSLIO *fslio, const char *units) { int unitcode=0; if (fslio==NULL) FSLIOERR("FslSetVoxUnits: Null pointer passed for FSLIO"); @@ -1384,7 +1400,7 @@ void FslSetVoxUnits(FSLIO *fslio, const char *units) } -void FslGetVoxUnits(FSLIO *fslio, char *units) +static void FslGetVoxUnits(FSLIO *fslio, char *units) { if (fslio==NULL) FSLIOERR("FslGetVoxUnits: Null pointer passed for FSLIO"); if (fslio->niftiptr!=NULL) { @@ -1394,6 +1410,7 @@ void FslGetVoxUnits(FSLIO *fslio, char *units) fprintf(stderr,"Warning:: Minc is not yet supported\n"); } } +#endif void FslSetTimeUnits(FSLIO *fslio, const char *units) { @@ -1461,7 +1478,7 @@ size_t FslGetDataType(FSLIO *fslio, short *t) if (fslio->mincptr!=NULL) { fprintf(stderr,"Warning:: Minc is not yet supported\n"); } - return (size_t) 8 * nbytepix; + return (size_t) 8 * (size_t)nbytepix; } @@ -2092,7 +2109,6 @@ FSLIO * FslReadHeader(char *fname) if (fslio->niftiptr == NULL) { FSLIOERR("FslReadHeader: error reading header information"); - return(NULL); } fslio->file_mode = FslGetReadFileType(fslio); @@ -2352,16 +2368,16 @@ double ***d3matrix(int zh, int yh, int xh) /** allocate pointers to slices */ - t=(double ***) malloc((size_t)((nslice)*sizeof(double**))); + t=(double ***) malloc(nslice*sizeof(double**)); if (!t) FSLIOERR("d3matrix: allocation failure"); /** allocate pointers for ydim */ - t[0]=(double **) malloc((size_t)((nslice*nrow)*sizeof(double*))); + t[0]=(double **) malloc(nslice*nrow*sizeof(double*)); if (!t[0]) FSLIOERR("d3matrix: allocation failure"); /** allocate the data blob */ - t[0][0]=(double *) malloc((size_t)((nslice*nrow*ncol)*sizeof(double))); + t[0][0]=(double *) malloc(nslice*nrow*ncol*sizeof(double)); if (!t[0][0]) FSLIOERR("d3matrix: allocation failure"); @@ -2402,20 +2418,20 @@ double ****d4matrix(int th, int zh, int yh, int xh) /** allocate pointers to vols */ - t=(double ****) malloc((size_t)((nvol)*sizeof(double***))); + t=(double ****) malloc((size_t)nvol*sizeof(double***)); if (!t) FSLIOERR("d4matrix: allocation failure"); /** allocate pointers to slices */ - t[0]=(double ***) malloc((size_t)((nvol*nslice)*sizeof(double**))); + t[0]=(double ***) malloc((size_t)nvol*(size_t)nslice*sizeof(double**)); if (!t[0]) FSLIOERR("d4matrix: allocation failure"); /** allocate pointers for ydim */ - t[0][0]=(double **) malloc((size_t)((nvol*nslice*nrow)*sizeof(double*))); + t[0][0]=(double **) malloc((size_t)nvol*(size_t)nslice*(size_t)nrow*sizeof(double*)); if (!t[0][0]) FSLIOERR("d4matrix: allocation failure"); /** allocate the data blob */ - t[0][0][0]=(double *) malloc((size_t)((nvol*nslice*nrow*ncol)*sizeof(double))); + t[0][0][0]=(double *) malloc((size_t)nvol*(size_t)nslice*(size_t)nrow*(size_t)ncol*sizeof(double)); if (!t[0][0][0]) FSLIOERR("d4matrix: allocation failure"); diff --git a/fsliolib/fslio.h b/fsliolib/fslio.h index d979fd48..ab1edbd5 100644 --- a/fsliolib/fslio.h +++ b/fsliolib/fslio.h @@ -206,6 +206,8 @@ FSL_API int FslReadRawHeader(void *buffer, const char* filename); /* simple creation and clone/copy operations */ FSL_API FSLIO *FslInit(void); +FSL_API void FslGetHdrImgNames(const char* filename, const FSLIO* fslio, + char** hdrname, char** imgname); FSL_API void FslInitHeader(FSLIO *fslio, short t, size_t x, size_t y, size_t z, size_t v, float vx, float vy, float vz, float tr, @@ -233,6 +235,7 @@ FSL_API void FslGetTimeUnits(FSLIO *fslio, char *units); FSL_API void FslSetDataType(FSLIO *fslio, short t); FSL_API size_t FslGetDataType(FSLIO *fslio, short *t); FSL_API int FslGetIntensityScaling(FSLIO *fslio, float *slope, float *intercept); +FSL_API void FslSetIntensityScaling(FSLIO *fslio, float slope, float intercept); FSL_API void FslSetIntent(FSLIO *fslio, short intent_code, float p1, float p2, float p3); FSL_API short FslGetIntent(FSLIO *fslio, short *intent_code, float *p1, float *p2, float *p3); diff --git a/nifti2/CMakeLists.txt b/nifti2/CMakeLists.txt index 6028291d..4564ed76 100644 --- a/nifti2/CMakeLists.txt +++ b/nifti2/CMakeLists.txt @@ -184,11 +184,64 @@ if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) if(TEST_INSTALL) add_test( NAME install_linking - COMMAND sh ${NIFTI_TEST_SCRIPT_DIR}/install_linking_test.sh ${CMAKE_MAKE_PROGRAM} + COMMAND sh ${NIFTI_TEST_SCRIPT_DIR}/install_linking_test.sh ${CMAKE_MAKE_PROGRAM} ${CMAKE_SOURCE_DIR} WORKING_DIRECTORY ${PROJECT_BINARY_DIR} ) endif() endif() + # A dim[0] outside 1..7 must be refused rather than used to index dim[]. + add_test( NAME ${TEST_PREFIX}_tool_disp_nim_bad_dim0 COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n2_bad_dim0.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_disp_nim_bad_dim0 PROPERTIES PASS_REGULAR_EXPRESSION "bad dim\\[0\\]" ) + + # A nifti_1_header is swapped at NIFTI-1 width whatever its magic claims. + add_test( NAME ${TEST_PREFIX}_tool_check_hdr_magic_n2 COMMAND $ -check_hdr -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_magic_n2.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_check_hdr_magic_n2 PROPERTIES PASS_REGULAR_EXPRESSION "header IS GOOD" ) + + # The tool's own header copies are swapped at their own width too. Both + # invocations rewrite the file, so each works on a copy. + add_test( NAME ${TEST_PREFIX}_tool_mod_hdr_magic_n2_setup COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_magic_n2.nii ${CMAKE_CURRENT_BINARY_DIR}/mod_magic_n2.nii ) + add_test( NAME ${TEST_PREFIX}_tool_mod_hdr_magic_n2 COMMAND $ -mod_hdr -mod_field descrip hello -overwrite -infiles ${CMAKE_CURRENT_BINARY_DIR}/mod_magic_n2.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_mod_hdr_magic_n2 PROPERTIES DEPENDS ${TEST_PREFIX}_tool_mod_hdr_magic_n2_setup ) + add_test( NAME ${TEST_PREFIX}_tool_swap_as_old_magic_n2_setup COMMAND ${CMAKE_COMMAND} -E copy ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_magic_n2.nii ${CMAKE_CURRENT_BINARY_DIR}/swap_magic_n2.nii ) + add_test( NAME ${TEST_PREFIX}_tool_swap_as_old_magic_n2 COMMAND $ -swap_as_old -overwrite -infiles ${CMAKE_CURRENT_BINARY_DIR}/swap_magic_n2.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_swap_as_old_magic_n2 PROPERTIES DEPENDS ${TEST_PREFIX}_tool_swap_as_old_magic_n2_setup ) + + # Dimensions whose product overflows the voxel count are refused. + add_test( NAME ${TEST_PREFIX}_tool_reject_nvox_overflow_n1 COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_overflow.nii ) + add_test( NAME ${TEST_PREFIX}_tool_reject_nvox_overflow_n2 COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n2_overflow.nii ) + add_test( NAME ${TEST_PREFIX}_tool_reject_volsize_overflow COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_volsize.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_reject_nvox_overflow_n1 ${TEST_PREFIX}_tool_reject_nvox_overflow_n2 + PROPERTIES PASS_REGULAR_EXPRESSION "dim\\[\\] overflows the voxel count" ) + set_tests_properties( ${TEST_PREFIX}_tool_reject_volsize_overflow + PROPERTIES PASS_REGULAR_EXPRESSION "dim\\[\\] and datatype overflow the volume size" ) + + # A repeated ASCII attribute releases the previous value, and the last + # one wins; the regex pins the surviving behavior. + add_test( NAME ${TEST_PREFIX}_tool_ascii_dup_attr COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/dup_attr.nia ) + set_tests_properties( ${TEST_PREFIX}_tool_ascii_dup_attr PROPERTIES PASS_REGULAR_EXPRESSION "dup2.nia" ) + + # Only compiled when the FSL name-conflict path is built. + if(FSLSTYLE_NAME_CONFLICTS) + add_test( NAME ${TEST_PREFIX}_ambiguous_hdrname + COMMAND ${CMAKE_COMMAND} -DTOOL=$ + -DDIR=${CMAKE_CURRENT_BINARY_DIR}/ambig + -P ${CMAKE_CURRENT_LIST_DIR}/ambiguous_hdrname.cmake ) + endif() + + if(UNIX AND NIFTI_SHELL_SCRIPT_TESTS) + # Reaches the failure paths that have to release the header before returning. + add_test( NAME ${TEST_PREFIX}_mod_hdr_errpaths COMMAND sh ${NIFTI_TEST_SCRIPT_DIR}/mod_header_errpaths.sh $ $ ) + endif() + + if(UNIX AND NIFTI_SHELL_SCRIPT_TESTS) + # Needs no external data, so it carries no NEEDS_DATA label. + add_test( NAME ${TEST_PREFIX}_nii_cmp_selftest COMMAND sh ${NIFTI_TEST_SCRIPT_DIR}/nii_cmp_selftest.sh ${NIFTI_TEST_SCRIPT_DIR} ) + endif() + + # Reading an ASCII header through the NIFTI-2 reader releases the image + # it built; the leak is what the memcheck and sanitizer legs observe. + add_test( NAME ${TEST_PREFIX}_tool_disp_hdr2_ascii COMMAND $ -disp_hdr2 -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/dup_attr.nia ) + unset(TEST_SUFFIX) unset(TEST_PREFIX) unset(TOOL_NAME) diff --git a/nifti2/ambiguous_hdrname.cmake b/nifti2/ambiguous_hdrname.cmake new file mode 100644 index 00000000..3353b6da --- /dev/null +++ b/nifti2/ambiguous_hdrname.cmake @@ -0,0 +1,30 @@ +# Drives the ambiguous-filename path, which must report the clash to its +# caller. A plain add_test cannot tell exit 1 from the abort it replaced. +file(REMOVE_RECURSE ${DIR}) +file(MAKE_DIRECTORY ${DIR}) + +execute_process(COMMAND ${TOOL} -make_im -new_dim 3 4 4 4 0 0 0 0 + -prefix ${DIR}/amb.nii RESULT_VARIABLE mk1) +execute_process(COMMAND ${TOOL} -make_im -new_dim 3 4 4 4 0 0 0 0 + -prefix ${DIR}/amb.nii.gz RESULT_VARIABLE mk2) +if(NOT mk1 STREQUAL "0" OR NOT mk2 STREQUAL "0") + message(FATAL_ERROR "could not create the ambiguous pair: ${mk1} ${mk2}") +endif() + +execute_process(COMMAND ${TOOL} -disp_hdr -infiles ${DIR}/amb + RESULT_VARIABLE rv ERROR_VARIABLE err OUTPUT_VARIABLE out) +if(NOT rv STREQUAL "1") + message(FATAL_ERROR "expected exit 1, got '${rv}'\n${err}") +endif() +if(NOT err MATCHES "Multiple possible filenames") + message(FATAL_ERROR "missing ambiguity diagnostic:\n${err}") +endif() + +# Unambiguous names must still resolve, so that returning NULL +# unconditionally could not pass. +file(REMOVE ${DIR}/amb.nii.gz) +execute_process(COMMAND ${TOOL} -disp_hdr -infiles ${DIR}/amb + RESULT_VARIABLE rv2 ERROR_VARIABLE err2 OUTPUT_VARIABLE out2) +if(NOT rv2 STREQUAL "0") + message(FATAL_ERROR "unambiguous name failed with '${rv2}'\n${err2}") +endif() diff --git a/nifti2/nifti2_io.c b/nifti2/nifti2_io.c index c8cc6b23..19dd1196 100644 --- a/nifti2/nifti2_io.c +++ b/nifti2/nifti2_io.c @@ -472,6 +472,9 @@ static const nifti_type_ele nifti_type_list[] = { }; /*---------------------------------------------------------------------------*/ +/* Defined in both libniftiio and libnifti2; linkage pending that duplication. */ +int nifti_fileexists(const char* fname); + /* prototypes for internal functions - not part of exported library */ /* extension routines */ @@ -987,7 +990,7 @@ static int nifti_load_NBL_bricks( nifti_image * nim , const int64_t * slist, } else { /* we have already read this sub-brick, just copy the previous one */ /* note that this works because they are sorted */ - memcpy(NBL->bricks[idest], NBL->bricks[sindex[c-1]], NBL->bsize); + memcpy(NBL->bricks[idest], NBL->bricks[sindex[c-1]], (size_t)(NBL->bsize)); } prev = isrc; /* in any case, note the now previous sub-brick */ @@ -1016,7 +1019,7 @@ static int nifti_alloc_NBL_mem(const nifti_image * nim, int64_t nbricks, } nbl->bsize = nim->nx * nim->ny * nim->nz * nim->nbyper; /* bytes */ - nbl->bricks = (void **)malloc(nbl->nbricks * sizeof(void *)); + nbl->bricks = (void **)malloc((size_t)nbl->nbricks * sizeof(void *)); if( ! nbl->bricks ){ fprintf(stderr,"** NIFTI NANM: failed to alloc %" PRId64 @@ -1025,7 +1028,7 @@ static int nifti_alloc_NBL_mem(const nifti_image * nim, int64_t nbricks, } for( c = 0; c < nbl->nbricks; c++ ){ - nbl->bricks[c] = malloc(nbl->bsize); + nbl->bricks[c] = malloc((size_t)(nbl->bsize)); if( ! nbl->bricks[c] ){ fprintf(stderr,"** NIFTI NANM: failed to alloc %" PRId64 " bytes for brick %" PRId64 "\n", nbl->bsize, c); @@ -1067,8 +1070,8 @@ static int nifti_copynsort(int64_t nbricks, const int64_t *blist, int64_t * stmp, * itmp; /* for ease of typing/reading */ int64_t c1, c2, spos, tmp; - *slist = (int64_t *)malloc(nbricks * sizeof(int64_t)); - *sindex = (int64_t *)malloc(nbricks * sizeof(int64_t)); + *slist = (int64_t *)malloc((size_t)nbricks * sizeof(int64_t)); + *sindex = (int64_t *)malloc((size_t)nbricks * sizeof(int64_t)); if( !*slist || !*sindex ){ fprintf(stderr,"** NIFTI NCS: failed to alloc %" PRId64 @@ -1281,13 +1284,13 @@ char *nifti_strdup(const char *str) { if( !str ) return NULL; /* allow calls passing NULL */ - size_t length = strlen(str); - char *dup = (char *)malloc(length + 1); + size_t length = strlen(str) + 1; + char *dup = (char *)malloc(length); /* check for failure */ - if( dup ) strcpy(dup, str); + if( dup ) strlcpy(dup, str, length); else fprintf(stderr,"** nifti_strdup: failed to alloc %zu bytes\n", - length+1); + length); return dup; } @@ -3525,7 +3528,7 @@ const char * nifti_find_file_extension( const char * name ) ext = name + len - 4; /* make manipulation copy, and possibly convert to lowercase */ - strcpy(extcopy, ext); + strlcpy(extcopy, ext, sizeof(extcopy)); if( g_opts.allow_upper_fext ) make_lowercase(extcopy); /* if it look like a basic extension, fail or return it */ @@ -3544,11 +3547,13 @@ const char * nifti_find_file_extension( const char * name ) ext = name + len - 7; /* make manipulation copy, and possibly convert to lowercase */ - strcpy(extcopy, ext); + strlcpy(extcopy, ext, sizeof(extcopy)); if( g_opts.allow_upper_fext ) make_lowercase(extcopy); /* go after .gz extensions using the modifiable strings */ - strcat(elist[0], extgz); strcat(elist[1], extgz); strcat(elist[2], extgz); + strlcat(elist[0], extgz, 8); + strlcat(elist[1], extgz, 8); + strlcat(elist[2], extgz, 8); if( compare_strlist(extcopy, elist, 3) >= 0 ) { if( is_mixedcase(ext) ) { @@ -3746,34 +3751,46 @@ char * nifti_findhdrname(const char* fname) make_uppercase(extzip); } - hdrname = (char *)calloc(sizeof(char),strlen(basename)+8); + size_t hdrnamelength = strlen(basename)+8; + hdrname = (char *)calloc(sizeof(char),hdrnamelength); if( !hdrname ){ fprintf(stderr,"** nifti_findhdrname: failed to alloc hdrname\n"); free(basename); return NULL; } - strcpy(hdrname,basename); - strcat(hdrname,elist[efirst]); + strlcpy(hdrname, basename, hdrnamelength); + strlcat(hdrname, elist[efirst], hdrnamelength); #ifdef FSLSTYLE if (nifti_fileexists(hdrname)) { - free(basename); - char *gzname = (char *)calloc(sizeof(char),strlen(hdrname)+8); - strcpy(gzname, hdrname); - strcat(gzname,extzip); + /* basename is read by the error message below, so it cannot be + freed here; gzname's allocation is used at once, so it has to be + checked; and a library reports an ambiguous name to its caller + rather than ending the process, which means every path out of + here now has to release what it holds. */ + size_t gznamelength = strlen(hdrname)+8; + char *gzname = (char *)calloc(sizeof(char),gznamelength); + if( !gzname ){ + fprintf(stderr,"** nifti_findhdrname: failed to alloc gzname\n"); + free(basename); free(hdrname); + return NULL; + } + strlcpy(gzname, hdrname, gznamelength); + strlcat(gzname,extzip,gznamelength); if (nifti_fileexists(gzname)) { fprintf(stderr,"Image Exception : Multiple possible filenames detected for basename (*.nii, *.nii.gz): %s\n", basename); - free(gzname); - exit(134); + free(gzname); free(basename); free(hdrname); + return NULL; } free(gzname); + free(basename); return hdrname; } #else if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif #ifdef HAVE_ZLIB - strcat(hdrname,extzip); + strlcat(hdrname, extzip, hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif @@ -3781,11 +3798,11 @@ char * nifti_findhdrname(const char* fname) efirst = 1 - efirst; - strcpy(hdrname,basename); - strcat(hdrname,elist[efirst]); + strlcpy(hdrname, basename, hdrnamelength); + strlcat(hdrname, elist[efirst], hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #ifdef HAVE_ZLIB - strcat(hdrname,extzip); + strlcat(hdrname, extzip, hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif @@ -3824,8 +3841,9 @@ char * nifti_findimgname(const char* fname , int nifti_type) /* check input file(s) for sanity */ if( !nifti_validfilename(fname) ) return NULL; - basename = nifti_makebasename(fname); - imgname = (char *)calloc(sizeof(char),strlen(basename)+8); + basename = nifti_makebasename(fname); + size_t imgnamelength = strlen(basename)+8; + imgname = (char *)calloc(sizeof(char),imgnamelength); if( !imgname ){ fprintf(stderr,"** nifti_findimgname: failed to alloc imgname\n"); free(basename); @@ -3843,8 +3861,8 @@ char * nifti_findimgname(const char* fname , int nifti_type) /* only valid extension for ASCII type is .nia, handle first */ if( nifti_type == NIFTI_FTYPE_ASCII ){ - strcpy(imgname,basename); - strcat(imgname,extnia); + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, extnia, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } } else { @@ -3858,21 +3876,21 @@ char * nifti_findimgname(const char* fname , int nifti_type) else if (nifti_type == NIFTI_FTYPE_NIFTI2_1) first = 0; else first = 1; /* should match .img */ - strcpy(imgname,basename); - strcat(imgname,elist[first]); + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, elist[first], imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #ifdef HAVE_ZLIB /* then also check for .gz */ - strcat(imgname,extzip); + strlcat(imgname, extzip, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #endif /* failed to find image file with expected extension, try the other */ - strcpy(imgname,basename); - strcat(imgname,elist[1-first]); /* can do this with only 2 choices */ + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, elist[1-first], imgnamelength); /* can do this with only 2 choices */ if (nifti_fileexists(imgname)) { free(basename); return imgname; } #ifdef HAVE_ZLIB /* then also check for .gz */ - strcat(imgname,extzip); + strlcat(imgname, extzip, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #endif } @@ -3912,12 +3930,13 @@ char * nifti_makehdrname(const char * prefix, int nifti_type, int check, if( !nifti_validfilename(prefix) ) return NULL; /* add space for extension, optional ".gz", and null char */ - iname = (char *)calloc(sizeof(char),strlen(prefix)+8); + size_t inamelength = strlen(prefix)+8; + iname = (char *)calloc(sizeof(char),inamelength); if( !iname ){ fprintf(stderr,"** NIFTI small malloc failure!\n"); return NULL; } - strcpy(iname, prefix); + strlcpy(iname, prefix, inamelength); /* use any valid extension */ if( (ext = nifti_find_file_extension(iname)) != NULL ){ @@ -3936,13 +3955,13 @@ char * nifti_makehdrname(const char * prefix, int nifti_type, int check, } } /* otherwise, make one up */ - else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_NIFTI2_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_ASCII ) strcat(iname, extnia); - else strcat(iname, exthdr); + else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_NIFTI2_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_ASCII ) strlcat(iname, extnia, inamelength); + else strlcat(iname, exthdr, inamelength); #ifdef HAVE_ZLIB /* if compression is requested, make sure of suffix */ - if( comp && (!ext || !strstr(iname,extgz)) ) strcat(iname,extgz); + if( comp && (!ext || !strstr(iname,extgz)) ) strlcat(iname, extgz, inamelength); #endif /* check for existence failure */ @@ -3987,12 +4006,13 @@ char * nifti_makeimgname(const char * prefix, int nifti_type, int check, if( !nifti_validfilename(prefix) ) return NULL; /* add space for extension, optional ".gz", and null char */ - iname = (char *)calloc(sizeof(char),strlen(prefix)+8); + size_t inamelength = strlen(prefix)+8; + iname = (char *)calloc(sizeof(char),inamelength); if( !iname ){ fprintf(stderr,"** NIFTI: small malloc failure!\n"); return NULL; } - strcpy(iname, prefix); + strlcpy(iname, prefix, inamelength); /* use any valid extension */ if( (ext = nifti_find_file_extension(iname)) != NULL ){ @@ -4011,13 +4031,13 @@ char * nifti_makeimgname(const char * prefix, int nifti_type, int check, } } /* otherwise, make one up */ - else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_NIFTI2_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_ASCII ) strcat(iname, extnia); - else strcat(iname, extimg); + else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_NIFTI2_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_ASCII ) strlcat(iname, extnia, inamelength); + else strlcat(iname, extimg, inamelength); #ifdef HAVE_ZLIB /* if compression is requested, make sure of suffix */ - if( comp && (!ext || !strstr(iname,extgz)) ) strcat(iname,extgz); + if( comp && (!ext || !strstr(iname,extgz)) ) strlcat(iname, extgz, inamelength); #endif /* check for existence failure */ @@ -4736,12 +4756,15 @@ nifti_image* nifti_convert_n1hdr2nim(nifti_1_header nhdr, const char * fname) * the qform_code will be zero, at which point you can check * analyze75_orient if you care to. */ - unsigned char c = *((char *)(&nhdr.qform_code)); + unsigned char c = *((unsigned char *)(&nhdr.qform_code)); nim->analyze75_orient = (analyze_75_orient_code)c; } if( doswap ) { if ( g_opts.debug > 3 ) disp_nifti_1_header("-d ni1 pre-swap: ", &nhdr); - swap_nifti_header( &nhdr , ni_ver ) ; + /* nhdr is a nifti_1_header, so swap it as one. ni_ver comes from the + magic string, and a magic of "n+2" would otherwise have + swap_nifti_header() treat these 348 bytes as a 540 byte header. */ + swap_nifti_header( &nhdr , ni_ver ? 1 : 0 ) ; } if ( g_opts.debug > 2 ) disp_nifti_1_header("-d nhdr2nim : ", &nhdr); @@ -4804,8 +4827,14 @@ nifti_image* nifti_convert_n1hdr2nim(nifti_1_header nhdr, const char * fname) nim->nv = nim->dim[6] = nhdr.dim[6]; nim->nw = nim->dim[7] = nhdr.dim[7]; - for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ) + /* the product of the dimensions becomes an allocation size, so refuse + the header rather than let it wrap */ + for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ){ + if( nhdr.dim[ii] > 0 && nim->nvox > INT64_MAX / nhdr.dim[ii] ){ + free(nim); ERREX("dim[] overflows the voxel count"); + } nim->nvox *= nhdr.dim[ii]; + } /**- set the type of data in voxels and how many bytes per voxel */ @@ -4814,6 +4843,11 @@ nifti_image* nifti_convert_n1hdr2nim(nifti_1_header nhdr, const char * fname) nifti_datatype_sizes( nim->datatype , &(nim->nbyper) , &(nim->swapsize) ) ; if( nim->nbyper == 0 ){ free(nim); ERREX("bad datatype"); } + /* nifti_get_volsize() multiplies these two */ + if( nim->nvox > INT64_MAX / nim->nbyper ){ + free(nim); ERREX("dim[] and datatype overflow the volume size"); + } + /**- set the grid spacings */ nim->dx = nim->pixdim[1] = nhdr.pixdim[1] ; @@ -5033,6 +5067,14 @@ nifti_image* nifti_convert_n2hdr2nim(nifti_2_header nhdr, const char * fname) ERREX("bad datatype") ; } + /* dim[0] is the number of dimensions and the loops below index dim[] + with it; the NIFTI-1 path gets this check from need_nhdr_swap() */ + if( nhdr.dim[0] < 0 || nhdr.dim[0] > 7 ) + { + free(nim); + ERREX("bad dim[0]") ; + } + if( nhdr.dim[1] <= 0 ) { free(nim); @@ -5074,8 +5116,14 @@ nifti_image* nifti_convert_n2hdr2nim(nifti_2_header nhdr, const char * fname) nim->nv = nim->dim[6] = nhdr.dim[6]; nim->nw = nim->dim[7] = nhdr.dim[7]; - for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ) + /* the product of the dimensions becomes an allocation size, so refuse + the header rather than let it wrap */ + for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ){ + if( nhdr.dim[ii] > 0 && nim->nvox > INT64_MAX / nhdr.dim[ii] ){ + free(nim); ERREX("dim[] overflows the voxel count"); + } nim->nvox *= nhdr.dim[ii]; + } /**- set the type of data in voxels and how many bytes per voxel */ @@ -5084,6 +5132,11 @@ nifti_image* nifti_convert_n2hdr2nim(nifti_2_header nhdr, const char * fname) nifti_datatype_sizes( nim->datatype , &(nim->nbyper) , &(nim->swapsize) ) ; if( nim->nbyper == 0 ){ free(nim); ERREX("bad datatype"); } + /* nifti_get_volsize() multiplies these two */ + if( nim->nvox > INT64_MAX / nim->nbyper ){ + free(nim); ERREX("dim[] and datatype overflow the volume size"); + } + /**- set the grid spacings */ nim->dx = nim->pixdim[1] = nhdr.pixdim[1] ; @@ -5364,7 +5417,9 @@ nifti_1_header * nifti_read_n1_hdr(const char * hname, int *swapped, int check) if( lswap ) { if ( g_opts.debug > 3 ) disp_nifti_1_header("-d nhdr pre-swap: ", &nhdr); - swap_nifti_header( &nhdr , NIFTI_VERSION(nhdr) ) ; + /* only sizeof(nifti_1_header) bytes were read, so swap as that; see + the same guard in nifti_convert_n1hdr2nim() */ + swap_nifti_header( &nhdr , NIFTI_VERSION(nhdr) ? 1 : 0 ) ; } if ( g_opts.debug > 2 ) disp_nifti_1_header("-d nhdr post-swap: ", &nhdr); @@ -5444,8 +5499,13 @@ nifti_2_header * nifti_read_n2_hdr(const char * hname, int * swapped, if( ! nim ) return NULL; hptr = (nifti_2_header *)malloc(sizeof(nifti_2_header)); + if( ! hptr ){ + fprintf(stderr,"** nifti_read_n2_hdr: failed to alloc nifti_2_header\n"); + nifti_image_free(nim); + return NULL; + } rv = nifti_convert_nim2n2hdr(nim, hptr); - free(nim); + nifti_image_free(nim); /* free(nim) leaked nim's filename strings */ if( rv ) { free(hptr); return NULL; } return hptr; @@ -5787,7 +5847,7 @@ void * nifti_read_header( const char *hname, int *nver, int check ) } /**- next read into nifti_1_header and determine nifti type */ - ii = (int)znzread(&n1hdr, 1, h1size, fp); + ii = (int)znzread(&n1hdr, 1, (size_t)h1size, fp); if( ii < (int)h1size ){ /* failure? */ if( g_opts.debug > 0 ){ @@ -5800,7 +5860,7 @@ void * nifti_read_header( const char *hname, int *nver, int check ) } /* find out what type of header we have */ - ni_ver = nifti_header_version((char *)&n1hdr, h1size); + ni_ver = nifti_header_version((char *)&n1hdr, (size_t)h1size); if( g_opts.debug > 2 ) fprintf(stderr,"-- %s: NIFTI version = %d\n", fname, ni_ver); @@ -5811,10 +5871,10 @@ void * nifti_read_header( const char *hname, int *nver, int check ) if ( ni_ver == 2 ) { if( g_opts.debug > 2 ) fprintf(stderr,"-- %s: copying and filling NIFTI-2 header...\n",fname); - memcpy(&n2hdr, &n1hdr, h1size); /* copy first part */ + memcpy(&n2hdr, &n1hdr, (size_t)h1size); /* copy first part */ remain = h2size - h1size; posn = (char *)&n2hdr + h1size; - ii = (int)znzread(posn, 1, remain, fp); /* read remaining part */ + ii = (int)znzread(posn, 1, (size_t)remain, fp); /* read remaining part */ if( ii < (int)remain) { LNI_FERR(fname,"short NIFTI-2 header read for file", hfile); znzclose(fp); free(hfile); return NULL; @@ -5827,24 +5887,24 @@ void * nifti_read_header( const char *hname, int *nver, int check ) /* allocate header space and return */ if( ni_ver == 0 || ni_ver == 1 ) { - hresult = malloc(h1size); + hresult = malloc((size_t)h1size); if( ! hresult ) { LNI_FERR(fname,"failed to alloc NIFTI-1 header for file", hname); return NULL; } - memcpy(hresult, (void *)&n1hdr, h1size); + memcpy(hresult, (void *)&n1hdr, (size_t)h1size); if ( check && ! nifti_hdr1_looks_good(hresult) ){ LNI_FERR(fname,"nifti_1_header looks bad for file", hname); return hresult; } } else if ( ni_ver == 2 ) { - hresult = malloc(h2size); + hresult = malloc((size_t)h2size); if( ! hresult ) { LNI_FERR(fname,"failed to alloc NIFTI-2 header for file", hname); return NULL; } - memcpy(hresult, &n2hdr, h2size); + memcpy(hresult, &n2hdr, (size_t)h2size); if ( check && ! nifti_hdr2_looks_good(hresult) ){ LNI_FERR(fname,"nifti_2_header looks bad for file", hname); @@ -5855,12 +5915,12 @@ void * nifti_read_header( const char *hname, int *nver, int check ) fprintf(stderr, "** %s: bad nifti header version %d\n", hname, ni_ver); /* return a nifti-1 header anyway */ - hresult = malloc(h1size); + hresult = malloc((size_t)h1size); if( ! hresult ) { LNI_FERR(fname,"failed to alloc NIFTI-?? header for file", hname); return NULL; } - memcpy(hresult, (void *)&n1hdr, h1size); + memcpy(hresult, (void *)&n1hdr, (size_t)h1size); } if( g_opts.debug > 1 ) @@ -5940,7 +6000,7 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) h2size = sizeof(nifti_2_header); /**- next read into nifti_1_header and determine nifti type */ - ii = (int)znzread(&n1hdr, 1, h1size, fp); + ii = (int)znzread(&n1hdr, 1, (size_t)h1size, fp); if( ii < (int)h1size ){ /* failure? */ if( g_opts.debug > 0 ){ @@ -5953,7 +6013,7 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) } /* find out what type of header we have */ - ni_ver = nifti_header_version((char *)&n1hdr, h1size); + ni_ver = nifti_header_version((char *)&n1hdr, (size_t)h1size); if( g_opts.debug > 2 ) fprintf(stderr,"-- %s: NIFTI version = %d\n", fname, ni_ver); @@ -5964,10 +6024,10 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) /* fill nifti-2 header and convert */ if( g_opts.debug > 2 ) fprintf(stderr,"-- %s: copying and filling NIFTI-2 header...\n",fname); - memcpy(&n2hdr, &n1hdr, h1size); /* copy first part */ + memcpy(&n2hdr, &n1hdr, (size_t)h1size); /* copy first part */ remain = h2size - h1size; posn = (char *)&n2hdr + h1size; - ii = (int)znzread(posn, 1, remain, fp); /* read remaining part */ + ii = (int)znzread(posn, 1, (size_t)remain, fp); /* read remaining part */ if( ii < (int)remain) { LNI_FERR(fname,"short NIFTI-2 header read for file", hfile); znzclose(fp); free(hfile); return NULL; @@ -5980,13 +6040,6 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) znzclose(fp); free(hfile); return NULL; } - #ifdef REJECT_COMPLEX - if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) { - fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname); - exit(13); - } - #endif - if( nim == NULL ){ znzclose( fp ) ; /* close the file */ if( g_opts.debug > 0 ) @@ -5995,6 +6048,13 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) return NULL; } + #ifdef REJECT_COMPLEX + if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) { + fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname); + exit(13); + } + #endif + if( g_opts.debug > 3 ){ fprintf(stderr,"+d nifti_image_read(), have nifti image:\n"); nifti_image_infodump(nim); @@ -6150,12 +6210,12 @@ nifti_image * nifti_read_ascii_image(znzFile fp, const char *fname, int flen, fname, slen); if( slen > 65530 ) slen = 65530 ; - sbuf = (char *)calloc(sizeof(char),slen+1) ; + sbuf = (char *)calloc(sizeof(char), (size_t)(slen+1)) ; if( !sbuf ){ fprintf(stderr,"** %s: failed to alloc %d bytes for sbuf",lfunc,65530); return NULL; } - znzread( sbuf , 1 , slen , fp ) ; + znzread( sbuf , 1 , (size_t)slen, fp ) ; nim = nifti_image_from_ascii( sbuf, &txt_size ) ; free( sbuf ) ; if( nim == NULL ){ LNI_FERR(lfunc,"failed nifti_image_from_ascii()",fname); @@ -6233,7 +6293,7 @@ static int nifti_read_extensions( nifti_image *nim, znzFile fp, int64_t remain ) return 0; } - count = znzread( extdr.extension, 1, 4, fp ); /* get extender */ + count = (int64_t)znzread( extdr.extension, 1, (size_t)4, fp ); /* get extender */ if( count < 4 ){ if( g_opts.debug > 1 ) @@ -6340,12 +6400,12 @@ static int nifti_add_exten_to_list( nifti1_extension * new_ext, nifti1_extension * tmplist; tmplist = *list; - *list = (nifti1_extension *)malloc(new_length * sizeof(nifti1_extension)); + *list = (nifti1_extension *)malloc((size_t)new_length * sizeof(nifti1_extension)); /* check for failure first */ if( ! *list ){ fprintf(stderr,"** NIFTI: failed to alloc %d ext structs (%zu bytes)\n", - new_length, new_length*sizeof(nifti1_extension)); + new_length, (size_t)new_length * sizeof(nifti1_extension)); if( !tmplist ) return -1; /* no old list to lose */ *list = tmplist; /* reset list to old one */ @@ -6354,7 +6414,7 @@ static int nifti_add_exten_to_list( nifti1_extension * new_ext, /* if an old list exists, copy the pointers and free the list */ if( tmplist ){ - memcpy(*list, tmplist, (new_length-1)*sizeof(nifti1_extension)); + memcpy(*list, tmplist, (size_t)(new_length-1)*sizeof(nifti1_extension)); free(tmplist); } @@ -6398,14 +6458,14 @@ static int nifti_fill_extension( nifti1_extension *ext, const char * data, ext->esize = esize; /* allocate esize-8 (maybe more than len), using calloc for fill */ - ext->edata = (char *)calloc(esize-8, sizeof(char)); + ext->edata = (char *)calloc((size_t)(esize-8),sizeof(char)); if( !ext->edata ){ fprintf(stderr,"** NIFTI NFE: failed to alloc %d bytes for extension\n", len); return -1; } - memcpy(ext->edata, data, len); /* copy the data, using len */ + memcpy(ext->edata, data, (size_t)len); /* copy the data, using len */ ext->ecode = ecode; /* set the ecode */ if( g_opts.debug > 2 ) @@ -6448,7 +6508,7 @@ static int nifti_read_next_extension( nifti1_extension * nex, nifti_image *nim, if( count != 2 || code == -1 ){ if( g_opts.debug > 2 ) fprintf(stderr,"-d current extension read failed\n"); - znzseek(fp, -4*count, SEEK_CUR); /* back up past any read */ + znzseek(fp, -4L*count, SEEK_CUR); /* back up past any read */ return 0; /* no extension, no error condition */ } @@ -6476,14 +6536,14 @@ static int nifti_read_next_extension( nifti1_extension * nex, nifti_image *nim, nex->ecode = code; size -= 8; /* subtract space for size and code in extension */ - nex->edata = (char *)malloc(size * sizeof(char)); + nex->edata = (char *)malloc((size_t)size * sizeof(char)); if( !nex->edata ){ fprintf(stderr,"** NIFTI: failed to allocate %d bytes for extension\n", size); return -1; } - count = (int)znzread(nex->edata, 1, size, fp); + count = (int)znzread(nex->edata, 1, (size_t)size, fp); if( count < size ){ if( g_opts.debug > 0 ) fprintf(stderr,"-d read only %d (of %d) bytes for extension\n", @@ -6800,7 +6860,7 @@ int nifti_image_load( nifti_image *nim ) if( nim->data == NULL ) { - nim->data = calloc(1,ntot) ; /* create image memory */ + nim->data = calloc(1, (size_t)ntot) ; /* create image memory */ if( nim->data == NULL ){ if( g_opts.debug > 0 ) fprintf(stderr,"** NIFTI: failed to alloc %d bytes for image data\n", @@ -6852,7 +6912,7 @@ int64_t nifti_read_buffer(znzFile fp, void* dataptr, int64_t ntot, return -1; } - ii = znzread( dataptr , 1 , ntot , fp ) ; /* data input */ + ii = (int64_t)znzread( dataptr , 1 , (size_t)ntot, fp ) ; /* data input */ /* if read was short, fail */ if( ii < ntot ){ @@ -6889,7 +6949,7 @@ if( g_opts.fix_floats ) case NIFTI_TYPE_FLOAT32: case NIFTI_TYPE_COMPLEX64:{ float *far = (float *)dataptr ; int64_t jj,nj ; - nj = ntot / sizeof(float) ; + nj = ntot / (int64_t)sizeof(float) ; for( jj=0 ; jj < nj ; jj++ ) /* count fixes 30 Nov 2004 [rickr] */ if( !IS_GOOD_FLOAT(far[jj]) ){ far[jj] = 0 ; @@ -6901,7 +6961,7 @@ if( g_opts.fix_floats ) case NIFTI_TYPE_FLOAT64: case NIFTI_TYPE_COMPLEX128:{ double *far = (double *)dataptr ; int64_t jj,nj ; - nj = ntot / sizeof(double) ; + nj = ntot / (int64_t)sizeof(double) ; for( jj=0 ; jj < nj ; jj++ ) /* count fixes 30 Nov 2004 [rickr] */ if( !IS_GOOD_FLOAT(far[jj]) ){ far[jj] = 0 ; @@ -7016,7 +7076,7 @@ int64_t nifti_write_buffer(znzFile fp, const void *buffer, int64_t numbytes) fprintf(stderr,"** ERROR: nifti_write_buffer: null file pointer\n"); return 0; } - ss = znzwrite( buffer , 1 , numbytes , fp ) ; + ss = (int64_t)znzwrite( buffer , 1 , (size_t)numbytes, fp ) ; return ss; } @@ -7342,7 +7402,7 @@ nifti_1_header * nifti_make_new_n1_header(const int64_t arg_dims[8], nifti_datatype_sizes( nhdr->datatype , &nbyper, &swapsize ); nhdr->bitpix = 8 * nbyper ; - strcpy(nhdr->magic, "n+1"); /* init to single file */ + strlcpy(nhdr->magic, "n+1", sizeof(nhdr->magic)); /* init to single file */ return nhdr; } @@ -7380,7 +7440,7 @@ nifti_image * nifti_make_new_nim(const int64_t dims[8], int datatype, fprintf(stderr,"+d nifti_make_new_nim, data_fill = %d\n",data_fill); if( data_fill ) { - nim->data = calloc(nim->nvox, nim->nbyper); + nim->data = calloc((size_t)(nim->nvox), (size_t)(nim->nbyper)); /* if we cannot allocate data, take ball and go home */ if( !nim->data ) { @@ -7474,8 +7534,8 @@ int nifti_convert_nim2n1hdr(const nifti_image * nim, nifti_1_header * hdr) if( nim->nifti_type > NIFTI_FTYPE_ANALYZE ){ /* then not ANALYZE */ - if( nim->nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcpy(nhdr.magic,"n+1") ; - else strcpy(nhdr.magic,"ni1") ; + if( nim->nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcpy(nhdr.magic, "n+1", sizeof(nhdr.magic)) ; + else strlcpy(nhdr.magic, "ni1", sizeof(nhdr.magic)) ; nhdr.pixdim[1] = (float)fabs(nhdr.pixdim[1]) ; nhdr.pixdim[2] = (float)fabs(nhdr.pixdim[2]) ; @@ -7693,8 +7753,8 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) if( nim_src->num_ext <= 0 ) return 0; - bytes = nim_src->num_ext * sizeof(nifti1_extension); /* I'm lazy */ - nim_dest->ext_list = (nifti1_extension *)malloc(bytes); + bytes = nim_src->num_ext * (int64_t)sizeof(nifti1_extension); /* I'm lazy */ + nim_dest->ext_list = (nifti1_extension *)malloc((size_t)bytes); if( !nim_dest->ext_list ){ fprintf(stderr,"** failed to allocate %d nifti1_extension structs\n", nim_src->num_ext); @@ -7710,7 +7770,7 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) fprintf(stderr,"+d dup'ing ext #%d of size %d (from size %d)\n", c, size, old_size); /* data length is size-8, as esize includes space for esize and ecode */ - data = (char *)calloc(size-8,sizeof(char)); /* maybe size > old */ + data = (char *)calloc((size_t)(size-8),sizeof(char)); /* maybe size > old */ if( !data ){ fprintf(stderr,"** NIFTI: failed to alloc %d bytes for extension\n", size); @@ -7722,7 +7782,7 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) nim_dest->ext_list[c].esize = size; nim_dest->ext_list[c].ecode = nim_src->ext_list[c].ecode; nim_dest->ext_list[c].edata = data; - memcpy(data, nim_src->ext_list[c].edata, old_size-8); + memcpy(data, nim_src->ext_list[c].edata, (size_t)(old_size-8)); nim_dest->num_ext++; } @@ -7856,13 +7916,13 @@ znzFile nifti_image_write_hdr_img2(nifti_image *nim, int write_opts, #ifdef PIGZ #ifdef HAVE_ZLIB -int doPigz2(nifti_image *nim, struct nifti_2_header nhdr, const nifti_brick_list * NBL) { +static int doPigz2(nifti_image *nim, struct nifti_2_header nhdr, const nifti_brick_list * NBL) { FILE *pigzPipe; char command[768]; - strcpy(command, "pigz" ); - strcat(command, " -n -f > \""); - strcat(command, nim->fname); - strcat(command, "\""); + strlcpy(command, "pigz", sizeof(command)); + strlcat(command, " -n -f > \"", sizeof(command)); + strlcat(command, nim->fname, sizeof(command)); + strlcat(command, "\"", sizeof(command)); #ifdef _MSC_VER if (( pigzPipe = _popen(command, "w")) == NULL) return -1; @@ -7888,13 +7948,13 @@ int doPigz2(nifti_image *nim, struct nifti_2_header nhdr, const nifti_brick_list return 0; } -int doPigz(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) { +static int doPigz(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) { FILE *pigzPipe; char command[768]; - strcpy(command, "pigz" ); - strcat(command, " -n -f > \""); - strcat(command, nim->fname); - strcat(command, "\""); + strlcpy(command, "pigz", sizeof(command)); + strlcat(command, " -n -f > \"", sizeof(command)); + strlcat(command, nim->fname, sizeof(command)); + strlcat(command, "\"", sizeof(command)); #ifdef _MSC_VER if (( pigzPipe = _popen(command, "w")) == NULL) return -1; @@ -8061,8 +8121,8 @@ static int nifti_image_write_engine(nifti_image *nim, int write_opts, /* write the header and extensions */ - if( nver == 2 ) ss = znzwrite(&n2hdr , 1 , hsize , fp); /* write header */ - else ss = znzwrite(&n1hdr , 1 , hsize , fp); /* write header */ + if( nver == 2 ) ss = znzwrite(&n2hdr , 1 , (size_t)hsize, fp); /* write header */ + else ss = znzwrite(&n1hdr , 1 , (size_t)hsize, fp); /* write header */ if( ss < hsize ){ LNI_FERR(func,"bad header write to output file",nim->fname); @@ -8408,7 +8468,7 @@ static char *escapize_string( const char * str ) default: lout++ ; break ; /* copy all other chars */ } } - out = (char *)calloc(1,lout) ; /* allocate output string */ + out = (char *)calloc(1, (size_t)lout) ; /* allocate output string */ if( !out ){ fprintf(stderr,"** NIFTI escapize_string: failed to alloc %d bytes\n", lout); @@ -8685,7 +8745,7 @@ char *nifti_image_to_ascii( const nifti_image *nim ) snprintf( buf+strlen(buf) , bufLen-strlen(buf) , "/>\n" ) ; /* XML-ish closer */ nbuf = (int)strlen(buf) ; - newbuf = (char *)realloc((void *)buf, nbuf+1); /* cut back to proper length */ + newbuf = (char *)realloc((void *)buf, (size_t)(nbuf+1)); /* cut back to proper length */ if( !newbuf ){ free(buf); fprintf(stderr,"** NIFTI NITA: failed to realloc %d bytes\n",nbuf+1); @@ -8730,7 +8790,7 @@ int nifti_short_order(void) /* determine this CPU's byte order */ put rhs string into nim->"nam" string, with field size = "sz" */ #define QSTR(nam,sz) if( strcmp(lhs,#nam) == 0 ) \ - strncpy(nim->nam,rhs,sz), nim->nam[sz-1]='\0' + memset(nim->nam, 0, sz), strlcpy(nim->nam,rhs,sz) /*---------------------------------------------------------------------------*/ /*! Take an XML-ish ASCII string and create a NIFTI image header to match. @@ -8797,7 +8857,7 @@ nifti_image *nifti_image_from_ascii( const char *str, int * bytes_read ) ii = spos+1 ; while( str[ii] != '\0' && str[ii] != '\'' ) ii++ ; nn = ii-spos-1 ; if( nn > 1023 ) nn = 1023 ; - memcpy(rhs,str+spos+1,nn) ; rhs[nn] = '\0' ; + memcpy(rhs,str+spos+1, (size_t)nn) ; rhs[nn] = '\0' ; spos = (str[ii] == '\'') ? ii+1 : ii ; } else { ii = sscanf( str+spos , "%1023s%n" , rhs , &nn ) ; spos += nn ; @@ -8823,9 +8883,11 @@ nifti_image *nifti_image_from_ascii( const char *str, int * bytes_read ) nim->nifti_type = NIFTI_FTYPE_NIFTI2_2 ; } else if( strcmp(lhs,"header_filename") == 0 ){ + free(nim->fname) ; /* the attribute may appear more than once */ nim->fname = nifti_strdup(rhs) ; } else if( strcmp(lhs,"image_filename") == 0 ){ + free(nim->iname) ; nim->iname = nifti_strdup(rhs) ; } else if( strcmp(lhs,"sto_xyz_matrix") == 0 ){ @@ -9316,7 +9378,7 @@ int64_t nifti_read_subregion_image( nifti_image * nim, for(i = 0; i < nim->ndim; i++) total_alloc_size *= region_size[i]; /* allocate buffer, if necessary */ - if(! *data) *data = malloc(total_alloc_size); + if(! *data) *data = malloc((size_t)total_alloc_size); if(! *data) { if(g_opts.debug > 1) @@ -9491,7 +9553,7 @@ static int rci_alloc_mem(void **data, const int64_t prods[8], int nprods, int nb " (%" PRId64 " x %d) bytes for collapsed image\n", size, size/nbyper, nbyper); - *data = malloc(size); /* actually allocate the memory */ + *data = malloc((size_t)size); /* actually allocate the memory */ if( ! *data ){ fprintf(stderr,"** NIFTI rci_am: failed to alloc %" PRId64 " bytes for data\n", size); @@ -9649,7 +9711,7 @@ int64_t * nifti_get_int64list( int64_t nvals , const char * str ) if( str[ipos] == ',' || ISEND(str[ipos]) ){ nout++ ; - subv_realloc = (int64_t *)realloc( (char *)subv , sizeof(int64_t)*(nout+1) ) ; + subv_realloc = (int64_t *)realloc( (char *)subv , sizeof(int64_t)*(size_t)(nout+1)) ; if( !subv_realloc ) { free(subv); fprintf(stderr,"** nifti_get_intlist: failed realloc of %" PRId64 @@ -9731,7 +9793,7 @@ int64_t * nifti_get_int64list( int64_t nvals , const char * str ) for( ii=ibot ; (ii-itop)*istep <= 0 ; ii += istep ){ nout++ ; - subv_realloc = (int64_t *)realloc( (char *)subv , sizeof(int64_t)*(nout+1) ) ; + subv_realloc = (int64_t *)realloc( (char *)subv , sizeof(int64_t)*(size_t)(nout+1)) ; if( !subv_realloc ) { free(subv); fprintf(stderr,"** nifti_get_intlist: failed realloc of %" PRId64 @@ -9782,7 +9844,7 @@ int * nifti_get_intlist( int nvals , const char * str ) } /* have a valid result, copy as ints */ - ilist = (int *)malloc((nints+1) * sizeof(int)); + ilist = (int *)malloc((size_t)(nints+1) * sizeof(int)); if( !ilist ) { fprintf(stderr,"** nifti_get_intlist: failed to alloc %" PRId64 " ints\n", nints); diff --git a/nifti2/nifti_regress_test/cmake_testscripts/c22_copy_image.sh b/nifti2/nifti_regress_test/cmake_testscripts/c22_copy_image.sh index a3abddc2..d2910d1b 100644 --- a/nifti2/nifti_regress_test/cmake_testscripts/c22_copy_image.sh +++ b/nifti2/nifti_regress_test/cmake_testscripts/c22_copy_image.sh @@ -11,6 +11,9 @@ DATA=$2 OUT_DATA=$(dirname ${DATA}) #Need to write to separate directory cd ${OUT_DATA} + +. "$(dirname "$0")/nii_cmp.sh" + # note the main input file and prefix for all output files infile=$DATA/e4.60005.nii.gz prefix=out.c22 @@ -42,7 +45,7 @@ ${NT} -copy_image -infile ${prefix}.0.i16.nii.gz \ ${NT} -copy_image -infile ${prefix}.1.i64.nii.gz \ -prefix ${prefix}.2.0.i16.nii.gz \ -convert2dtype NIFTI_TYPE_INT16 -convert_verify -if cmp ${prefix}.0.i16.nii.gz ${prefix}.2.0.i16.nii.gz +if nii_cmp ${prefix}.0.i16.nii.gz ${prefix}.2.0.i16.nii.gz then echo "" else @@ -57,7 +60,7 @@ ${NT} -cbl -infile ${prefix}.0.i16.nii.gz \ ${NT} -copy_image -infile ${prefix}.1.f32.nii.gz \ -prefix ${prefix}.2.1.i16.nii.gz \ -convert2dtype NIFTI_TYPE_INT16 -convert_fail_choice warn -if cmp ${prefix}.0.i16.nii.gz ${prefix}.2.1.i16.nii.gz +if nii_cmp ${prefix}.0.i16.nii.gz ${prefix}.2.1.i16.nii.gz then echo "" else diff --git a/nifti2/nifti_regress_test/cmake_testscripts/install_linking_test.sh b/nifti2/nifti_regress_test/cmake_testscripts/install_linking_test.sh index dccf0fe9..89dcaeaf 100755 --- a/nifti2/nifti_regress_test/cmake_testscripts/install_linking_test.sh +++ b/nifti2/nifti_regress_test/cmake_testscripts/install_linking_test.sh @@ -10,6 +10,18 @@ else export BUILD_TOOL=$1 fi +# Where the project source lives. This used to be hard-coded as +# ../../nifti_clib relative to the build directory, which only resolved +# when the build tree happened to be a sibling of a source tree named +# exactly "nifti_clib" -- so the test failed for an in-tree build, for a +# build directory named anything else, and on CI. +if [ $# -lt 2 ] +then + echo Missing source directory + exit 1 +fi +SRC_DIR=$2 + # Set variables for local install export DESTDIR=installed export PATH="$PWD/$DESTDIR/usr/local/bin:$PATH" @@ -28,7 +40,7 @@ cd downstream_example cmake \ -G 'Unix Makefiles' \ -DCMAKE_MODULE_PATH=../installed/usr/local/share \ - ../../nifti_clib/real_easy/minimal_example_of_downstream_usage + "${SRC_DIR}/real_easy/minimal_example_of_downstream_usage" make echo Success diff --git a/nifti2/nifti_regress_test/cmake_testscripts/mod_header_errpaths.sh b/nifti2/nifti_regress_test/cmake_testscripts/mod_header_errpaths.sh new file mode 100755 index 00000000..7ef0fd88 --- /dev/null +++ b/nifti2/nifti_regress_test/cmake_testscripts/mod_header_errpaths.sh @@ -0,0 +1,50 @@ +#!/bin/sh +# Drive the two failure paths in nifti_tool's header modification, where +# the header and the duplicated name have to be released before returning. +# Both invocations are expected to fail, so a nonzero status cannot be the +# assertion: under a sanitizer it is also what a leak report exits with. +# The check is that the tool reported the failure and said nothing else. +# usage: mod_header_errpaths.sh [nifti1_tool] +NT=$1 +NT1=$2 +[ -x "$NT" ] || { echo "usage: $0 [nifti1_tool]"; exit 1; } + +if [ "$(id -u)" = "0" ]; then + echo "skipping: a read-only directory does not stop root" + exit 0 +fi + +tmp=$(mktemp -d) || exit 1 +trap 'chmod 755 "$tmp/ro" 2>/dev/null; rm -rf "$tmp"' EXIT +cd "$tmp" || exit 1 + +$NT -make_im -new_dim 3 4 4 4 0 0 0 0 -prefix anat0.nii || exit 1 +mkdir ro && chmod 555 ro + +# A. the write fails, so both the header and the duplicated name are held +out=$($NT -mod_hdr2 -prefix ro/anat1 -infiles anat0.nii \ + -mod_field qoffset_x -17.325 2>&1) +if [ $? -eq 0 ]; then + echo "FAIL: writing into a read-only directory succeeded" + echo "$out" + exit 1 +fi +case "$out" in + *Sanitizer*) echo "FAIL: sanitizer report on the write path"; echo "$out"; exit 1;; +esac + +# B. the duplication fails, so the header alone is held +if [ -n "$NT1" ] && [ -x "$NT1" ]; then + out=$($NT1 -mod_hdr -prefix ro/x1 -infiles anat0.nii \ + -mod_field qoffset_x -17.325 2>&1) + if [ $? -eq 0 ]; then + echo "FAIL: nifti1_tool accepted a NIFTI-2 input" + echo "$out" + exit 1 + fi + case "$out" in + *Sanitizer*) echo "FAIL: sanitizer report on the duplication path"; echo "$out"; exit 1;; + esac +fi + +echo "mod_header error paths reached" diff --git a/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp.sh b/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp.sh new file mode 100644 index 00000000..a232c21f --- /dev/null +++ b/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp.sh @@ -0,0 +1,15 @@ +# Compare two NIfTI files by content rather than by compressed bytes. +# +# gzip output is not reproducible across zlib implementations: zlib-ng, +# which Arch, CachyOS and other current distributions ship as the system +# zlib, encodes the same input differently from stock zlib. Comparing +# the .gz files directly therefore fails on those systems even though the +# image data round-trips perfectly. Decompress first and compare that. +nii_cmp() { + if [ "${1##*.}" = "gz" ]; then + gzip -dc "$1" > "$1.raw" && gzip -dc "$2" > "$2.raw" || return 1 + cmp "$1.raw" "$2.raw" + return $? + fi + cmp "$1" "$2" +} diff --git a/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp_selftest.sh b/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp_selftest.sh new file mode 100755 index 00000000..2eecaada --- /dev/null +++ b/nifti2/nifti_regress_test/cmake_testscripts/nii_cmp_selftest.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# Verify nii_cmp compares NIfTI content rather than compressed bytes. +# +# gzip -1 and gzip -9 encode the same input differently, which manufactures +# the byte difference locally instead of waiting for a runner whose system +# zlib happens to be zlib-ng. +# usage: nii_cmp_selftest.sh +. "$1/nii_cmp.sh" + +tmp=$(mktemp -d) || exit 1 +trap 'rm -rf "$tmp"' EXIT +cd "$tmp" || exit 1 + +head -c 65536 /dev/urandom > payload +gzip -1 -c payload > a.nii.gz +gzip -9 -c payload > b.nii.gz +head -c 65536 /dev/urandom > other +gzip -c other > c.nii.gz + +if cmp -s a.nii.gz b.nii.gz; then + echo "SKIP: this gzip encodes -1 and -9 identically, nothing to compare" + exit 0 +fi + +nii_cmp a.nii.gz b.nii.gz || { + echo "FAIL: identical content reported different"; exit 1; } +nii_cmp a.nii.gz c.nii.gz >/dev/null 2>&1 && { + echo "FAIL: differing content reported same"; exit 1; } +cp payload d.nii && cp payload e.nii +nii_cmp d.nii e.nii || { echo "FAIL: uncompressed path broken"; exit 1; } + +echo "nii_cmp selftest passed" diff --git a/nifti2/nifti_tool.c b/nifti2/nifti_tool.c index ac249bf8..48e9fb53 100644 --- a/nifti2/nifti_tool.c +++ b/nifti2/nifti_tool.c @@ -864,7 +864,7 @@ int fill_cmd_string( nt_opts * opts, int argc, const char * argv[]) return 1; } cp = opts->command + len; - remain -= len; + remain -= (size_t)len; /* get the rest, with special attention to input files */ for( int ac = 1; ac < argc; ac++ ) @@ -885,11 +885,11 @@ int fill_cmd_string( nt_opts * opts, int argc, const char * argv[]) if( has_space ) len = snprintf(cp, remain, " '%s'", argv[ac]); else len = snprintf(cp, remain, " %s", argv[ac]); - if( len < 0 || len >= remain ) { + if( len < 0 || (size_t)len >= remain ) { fprintf(stderr,"FCS: error parsing command, continuing...\n"); return 1; } - remain -= len; + remain -= (size_t)len; /* infiles is okay, but after the *next* argument, we may skip files */ /* (danger, will robinson! hack alert!) */ @@ -922,7 +922,7 @@ int add_int(int_list * ilist, int val) { if( ilist->len == 0 ) ilist->list = NULL; /* just to be safe */ ilist->len++; - ilist->list = (int *)realloc(ilist->list,ilist->len*sizeof(int)); + ilist->list = (int *)realloc(ilist->list, (size_t)ilist->len * sizeof(int)); if( ! ilist->list ){ fprintf(stderr,"** failed to alloc %d (int *) elements\n",ilist->len); return -1; @@ -943,7 +943,7 @@ int add_string(str_list * slist, const char * str) { if( slist->len == 0 ) slist->list = NULL; /* just to be safe */ slist->len++; - slist->list = (const char **)realloc(slist->list,slist->len*sizeof(char *)); + slist->list = (const char **)realloc(slist->list, (size_t)slist->len * sizeof(char *)); if( ! slist->list ){ fprintf(stderr,"** failed to alloc %d (char *) elements\n",slist->len); return -1; @@ -2284,6 +2284,7 @@ int act_add_exts( nt_opts * opts ) opts->etypes.list[ec]); if( nifti_add_extension(nim, ext, elen, opts->etypes.list[ec]) ){ + free(edata); /* may hold the file contents read just above */ nifti_image_free(nim); return 1; } @@ -2357,17 +2358,17 @@ static char * read_file_text(const char * filename, int * length) /* allocate the bytes, and fill them with the file contents */ - text = (char *)malloc(len64 * sizeof(char)); + text = (char *)malloc((size_t)len64 * sizeof(char)); if( !text ) { fprintf(stderr,"** RFT: failed to allocate %" PRId64 " bytes\n", len64); fclose(fp); return NULL; } - bytes = fread(text, sizeof(char), len64, fp); + bytes = fread(text, sizeof(char), (size_t)len64, fp); fclose(fp); /* in any case */ - if( bytes != len64 ) { + if( bytes != (size_t)len64 ) { fprintf(stderr,"** RFT: read only %zu of %" PRId64 " bytes from %s\n", bytes, len64, filename); free(text); @@ -2561,7 +2562,7 @@ int remove_ext_list( nifti_image * nim, const char ** elist, int len ) if( g_debug > 2 ) fprintf(stderr,"+d removing %d exts from '%s'\n", len, nim->fname ); - if( ! (marks = (int *)calloc(nim->num_ext, sizeof(int))) ) { + if( ! (marks = (int *)calloc((size_t)(nim->num_ext),sizeof(int))) ) { fprintf(stderr,"** failed to alloc %d marks\n",nim->num_ext); return -1; } @@ -2843,7 +2844,7 @@ int act_diff_nims( nt_opts * opts ) if( ! nim0 ) return 1; /* errors have been printed */ nim1 = nt_image_read(opts, opts->infiles.list[1], 0, 0); - if( ! nim1 ){ free(nim0); return 1; } + if( ! nim1 ){ nifti_image_free(nim0); return 1; } if( g_debug > 1 ) fprintf(stderr,"\n-d nifti_image diffs between '%s' and '%s'...\n", @@ -3359,6 +3360,7 @@ int act_mod_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } @@ -3369,6 +3371,7 @@ int act_mod_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3377,6 +3380,8 @@ int act_mod_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3386,7 +3391,11 @@ int act_mod_hdrs( nt_opts * opts ) nifti_image_free(nim); } else if ( swap ) - swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr)); + /* nhdr is a nifti_1_header, so swap it as one. NIFTI_VERSION() + reads the magic string, and a magic of "n+2" would otherwise + have swap_nifti_header() treat these 348 bytes as a 540 byte + header. ni_ver 0 and 1 are both 348 byte layouts. */ + swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr) ? 1 : 0); /* if all is well, overwrite header in fname dataset */ (void)write_hdr_to_file(nhdr, fname); /* errors printed in function */ @@ -3477,6 +3486,7 @@ int act_mod_hdr2s( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -3486,6 +3496,7 @@ int act_mod_hdr2s( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3494,6 +3505,8 @@ int act_mod_hdr2s( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3503,7 +3516,9 @@ int act_mod_hdr2s( nt_opts * opts ) nifti_image_free(nim); } else if ( swap ) - swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr)); + /* nhdr is a nifti_2_header; use the explicit version rather than + the magic, which could claim "n+1" and swap 540 bytes as 348 */ + swap_nifti_header(nhdr, 2); /* if all is well, overwrite header in fname dataset */ (void)write_hdr2_to_file(nhdr, fname); /* errors printed in function */ @@ -3604,8 +3619,12 @@ int act_swap_hdrs( nt_opts * opts ) swap_nifti_header(nhdr, 0); /* undo ANALYZE */ swap_nifti_header(nhdr, 1); /* swap NIFTI */ } else if ( opts->swap_old ) { - /* undo whichever was done and apply the old way */ - swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr)); + /* undo whichever was done and apply the old way. As above, + nhdr is a nifti_1_header, so it must not be swapped as a + 540 byte NIFTI-2 header just because its magic says "n+2". + old_swap_nifti_header() takes a nifti_1_header and a + boolean, so it needs no such guard. */ + swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr) ? 1 : 0); old_swap_nifti_header(nhdr, NIFTI_VERSION(*nhdr)); } @@ -3623,6 +3642,7 @@ int act_swap_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -3632,6 +3652,7 @@ int act_swap_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -3640,6 +3661,8 @@ int act_swap_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3905,7 +3928,8 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((short *)((char *)basep + field->offset))[fc] = (short)val; + { const int16_t sval = (int16_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(sval), &sval,sizeof(sval)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3924,7 +3948,8 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int *)((char *)basep + field->offset))[fc] = val; + { const int32_t ival = (int32_t)val; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(ival), &ival,sizeof(ival)); } if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %d\n", fc, field->name, val); @@ -3944,7 +3969,7 @@ int modify_field(void * basep, field_s * field, const char * data) fc,field->len); return 1; } - ((int64_t *)((char *)basep + field->offset))[fc] = v64; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(v64), &v64,sizeof(v64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %" PRId64 "\n", fc, field->name, v64); @@ -3964,7 +3989,7 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((float *)((char *)basep + field->offset))[fc] = fval; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(fval), &fval,sizeof(fval)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, fval); @@ -3985,7 +4010,7 @@ int modify_field(void * basep, field_s * field, const char * data) return 1; } /* otherwise, we're good */ - ((double *)((char *)basep + field->offset))[fc] = f64; + memcpy((char *)basep + field->offset + (size_t)fc * sizeof(f64), &f64,sizeof(f64)); if( g_debug > 1 ) fprintf(stderr,"+d setting posn %d of '%s' to %f\n", fc, field->name, f64); @@ -3997,10 +4022,10 @@ int modify_field(void * basep, field_s * field, const char * data) case NT_DT_STRING: { char * dest = (char *)basep + field->offset; - nchars = dataLength; - strncpy(dest, data, field->len); + nchars = (int)dataLength; + strncpy(dest, data, (size_t)(field->len)); if( nchars < field->len ) /* clear the rest */ - memset(dest+nchars, '\0', field->len-nchars); + memset(dest+nchars, '\0', (size_t)(field->len-nchars)); } break; } @@ -4163,7 +4188,7 @@ static int convert_NBL_data(nifti_brick_list * NBL, int old_type, int new_type, nifti_datatype_sizes(new_type, &nbyper, NULL); NBLnew.bsize = nbvals * nbyper; NBLnew.nbricks = NBL->nbricks; - NBLnew.bricks = (void **)calloc(NBLnew.nbricks, sizeof(void *)); + NBLnew.bricks = (void **)calloc((size_t)NBLnew.nbricks, (size_t)(sizeof(void *))); if( ! NBLnew.bricks ) { fprintf(stderr,"** cNBLd: failed to allocate %" PRId64 " void pointers\n", NBLnew.nbricks); @@ -4255,7 +4280,7 @@ static int convert_raw_data(void ** retdata, void * olddata, int old_type, /* allocate new memory (calloc, in case of partial filling) */ nifti_datatype_sizes(new_type, &nbyper, NULL); /* get nbyper */ - newdata = calloc(nvox, nbyper); + newdata = calloc((size_t)nvox, (size_t)nbyper); if( !newdata ) { fprintf(stderr,"** failed to alloc for %" PRId64 " %s elements\n", nvox, typestr); @@ -6098,8 +6123,7 @@ int fill_field( field_s * fp, int type, int offset, int num, const char * name ) fp->size = 1; /* init before check */ fp->len = num; - strncpy(fp->name, name, sizeof(fp->name)); - fp->name[sizeof(fp->name) - 1] = 0; + strlcpy(fp->name, name, sizeof(fp->name)); switch( type ){ case DT_UNKNOWN: @@ -6248,7 +6272,7 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h int len; /* start by sucking the pointer stored here */ - sp = *(char **)((char *)str + fp->offset); + memcpy(&sp, (const char *)str + fp->offset, sizeof(sp)); if( ! sp ){ fprintf(stdout,"(NULL)\n"); break; } /* anything? */ @@ -6262,7 +6286,9 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h else if( *sp && !isprint(*sp) ) /* if no termination, it's bad */ fprintf(stdout,"(non-printable string)\n"); else /* woohoo! a good string */ - fprintf(stdout,"'%.40s'\n",*(char **)((char *)str + fp->offset)); + { char * cp; + memcpy(&cp, (const char *)str + fp->offset, sizeof(cp)); + fprintf(stdout,"'%.40s'\n", cp); } break; } @@ -6271,7 +6297,7 @@ int disp_field(const char *mesg, field_s *fieldp, void * str, int nfields, int h nifti1_extension * extp; /* yank the address sitting there into extp */ - extp = *(nifti1_extension **)((char *)str + fp->offset); + memcpy(&extp, (const char *)str + fp->offset, sizeof(extp)); /* the user may use -disp_exts to display all of them */ if( extp ) disp_nifti1_extension(NULL, extp, 6); @@ -6332,8 +6358,8 @@ int diff_field(field_s *fieldp, void * str0, void * str1, int nfields) { nifti1_extension * ext0, * ext1; - ext0 = *(nifti1_extension **)((char *)str0 + fp->offset); - ext1 = *(nifti1_extension **)((char *)str1 + fp->offset); + memcpy(&ext0, (const char *)str0 + fp->offset, sizeof(ext0)); + memcpy(&ext1, (const char *)str1 + fp->offset, sizeof(ext1)); if( ! ext0 && ! ext1 ) break; /* continue on */ @@ -7611,7 +7637,7 @@ nifti_image * nt_read_bricks(nt_opts * opts, char * fname, int len, /* now populate NBL (can be based only on len and nim) */ NBL->nbricks = len; NBL->bsize = nim->nbyper * nim->nx * nim->ny * nim->nz; - NBL->bricks = (void **)calloc(NBL->nbricks, sizeof(void *)); + NBL->bricks = (void **)calloc((size_t)(NBL->nbricks), (size_t)(sizeof(void *))); if( !NBL->bricks ){ fprintf(stderr,"** NRB: failed to alloc %" PRId64 " pointers\n", NBL->nbricks); @@ -7626,7 +7652,7 @@ nifti_image * nt_read_bricks(nt_opts * opts, char * fname, int len, /* now allocate the data pointers */ for( c = 0; c < len; c++ ) { - NBL->bricks[c] = calloc(1, NBL->bsize); + NBL->bricks[c] = calloc(1, (size_t)(NBL->bsize)); if( !NBL->bricks[c] ){ fprintf(stderr, "** NRB: failed to alloc brick %d of %" PRId64 " bytes\n", diff --git a/nifti2/nifti_tool.h b/nifti2/nifti_tool.h index d548f66d..ba57369d 100644 --- a/nifti2/nifti_tool.h +++ b/nifti2/nifti_tool.h @@ -108,10 +108,10 @@ typedef struct{ #define NT_NIM_NUM_FIELDS 63 /* in the nifti_image struct */ #define NT_HDR_TIME_NFIELDS 8 /* num slice timing fields in hdr */ #define NT_NIM_TIME_NFIELDS 11 /* num slice timing fields in nim */ -#define NT_DT_STRING -0xfff /* some strange number to abuse... */ -#define NT_DT_POINTER -0xfef /* some strange number to abuse... */ -#define NT_DT_CHAR_PTR -0xfee /* another... */ -#define NT_DT_EXT_PTR -0xfed /* and another... */ +#define NT_DT_STRING (-0xfff) /* some strange number to abuse... */ +#define NT_DT_POINTER (-0xfef) /* some strange number to abuse... */ +#define NT_DT_CHAR_PTR (-0xfee) /* another... */ +#define NT_DT_EXT_PTR (-0xfed) /* and another... */ typedef struct { int type; /* one of the DT_* types from nifti1.h */ @@ -142,7 +142,7 @@ typedef struct { dtype * pd = dptr; \ stype * ps = sptr; \ int64_t index; \ - for(index=0; index ) + foreach(DISTRIBUTION CORREL TTEST FTEST ZSCORE CHISQ BETA BINOM GAMMA POISSON NORMAL FTEST_NONC CHISQ_NONC LOGISTIC LAPLACE UNIFORM TTEST_NONC WEIBULL CHI INVGAUSS EXTVAL PVAL LOGPVAL LOG10PVAL ) add_test( NAME ${NIFTI_PACKAGE_PREFIX}nifti_stats_${DISTRIBUTION}_test COMMAND $ 0:4:1 ${DISTRIBUTION}) add_test( NAME q${NIFTI_PACKAGE_PREFIX}nifti_stats_${DISTRIBUTION}_test COMMAND $ -q 0:4:1 ${DISTRIBUTION}) diff --git a/nifticdf/nifticdf.c b/nifticdf/nifticdf.c index bdab933f..e97c9317 100644 --- a/nifticdf/nifticdf.c +++ b/nifticdf/nifticdf.c @@ -1892,7 +1892,7 @@ static double T5,T6,T7,T8,T9,T10,T12,T13; /* Check arguments */ - if(!(*which < 1 && *which > 4)) goto S30; + if(!(*which < 1 || *which > 4)) goto S30; if(!(*which < 1)) goto S10; *bound = 1.0e0; goto S20; @@ -11044,17 +11044,21 @@ char const * const inam[]={ NULL , NULL , int nifti_intent_code( const char *name ) { - char *unam , *upt ; - int ii ; + if( name == NULL || *name == '\0' ) + return -1 ; - if( name == NULL || *name == '\0' ) return -1 ; - - unam = (char *)malloc(strlen(name)+1); - strcpy(unam,name); - for( upt=unam ; *upt != '\0' ; upt++ ) *upt = (char)toupper(*upt) ; + size_t size = strlen(name)+1; + char *unam = (char *)malloc(size); + if (!unam) + return -1 ; + strlcpy(unam,name,size); + for( char *upt=unam ; *upt != '\0' ; upt++ ) + *upt = (char)toupper(*upt) ; + int ii ; for( ii=NIFTI_FIRST_STATCODE ; ii <= NIFTI_LAST_STATCODE ; ii++ ) - if( strcmp(inam[ii],unam) == 0 ) break ; + if( strcmp(inam[ii],unam) == 0 ) + break ; free(unam) ; return (ii <= NIFTI_LAST_STATCODE) ? ii : -1 ; diff --git a/nifticdf/nifticdf_range_test.c b/nifticdf/nifticdf_range_test.c new file mode 100644 index 00000000..63e607d6 --- /dev/null +++ b/nifticdf/nifticdf_range_test.c @@ -0,0 +1,50 @@ +/* cdfbin must reject an out-of-range "which" selector the way its ten + siblings in nifticdf.c do: *status = -1 and *bound = the limit. */ + +#include +#include "nifticdf.h" + +static int check(int which, double expected_bound) +{ + int status = 999; + double p = 0.5, q = 0.5, s = 2.0, xn = 5.0, pr = 0.5, ompr = 0.5; + double bound = -999.0; + + cdfbin(&which, &p, &q, &s, &xn, &pr, &ompr, &status, &bound); + + if( status != -1 || bound != expected_bound ) { + fprintf(stderr, "** cdfbin(which=%d): status=%d bound=%g," + " expected status=-1 bound=%g\n", + which, status, bound, expected_bound); + return 1; + } + printf("cdfbin(which=%d): status=%d bound=%g\n", which, status, bound); + return 0; +} + +int main(void) +{ + int errs = 0; + + errs += check(9, 4.0); /* above the legal range */ + errs += check(0, 1.0); /* below the legal range */ + + /* a legal selector must still compute, not report a range error */ + { + int which = 1, status = 999; + double p = 0.0, q = 0.0, s = 2.0, xn = 5.0, pr = 0.5, ompr = 0.5; + double bound = -999.0; + + cdfbin(&which, &p, &q, &s, &xn, &pr, &ompr, &status, &bound); + if( status != 0 ) { + fprintf(stderr, "** cdfbin(which=1): status=%d, expected 0\n", status); + errs++; + } else { + printf("cdfbin(which=1): status=0 p=%g\n", p); + } + } + + if( errs ) { fprintf(stderr, "** %d failure(s)\n", errs); return 1; } + printf("nifticdf range test passed\n"); + return 0; +} diff --git a/niftilib/CMakeLists.txt b/niftilib/CMakeLists.txt index 09d522d2..23461e87 100644 --- a/niftilib/CMakeLists.txt +++ b/niftilib/CMakeLists.txt @@ -40,6 +40,13 @@ if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) add_executable(nifti_first_test_program nifti_tester001.c) target_link_libraries(nifti_first_test_program ${NIFTI_PACKAGE_PREFIX}niftiio ) + add_executable(${NIFTI_PACKAGE_PREFIX}nifti_short_read_test nifti_short_read_test.c) + target_link_libraries(${NIFTI_PACKAGE_PREFIX}nifti_short_read_test ${NIFTI_NIFTILIB_NAME}) + add_test( + NAME nifti_short_read_rejected + COMMAND $ + ) + add_executable(nifti_second_test_program nifti_tester002.c) target_link_libraries(nifti_second_test_program ${NIFTI_PACKAGE_PREFIX}niftiio ) @@ -148,6 +155,25 @@ if(NIFTI_BUILD_TESTING AND NIFTI_BUILD_APPLICATIONS) ) #==END NIFTI1 and NIFTI2 common tests ============================================ + # The same guards in the NIFTI-1 library, reached through nifti1_tool. + add_test( NAME ${TEST_PREFIX}_tool_reject_nvox_overflow COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_overflow.nii ) + add_test( NAME ${TEST_PREFIX}_tool_reject_volsize_overflow COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/n1_volsize.nii ) + set_tests_properties( ${TEST_PREFIX}_tool_reject_nvox_overflow + PROPERTIES PASS_REGULAR_EXPRESSION "dim\\[\\] overflows the voxel count" ) + set_tests_properties( ${TEST_PREFIX}_tool_reject_volsize_overflow + PROPERTIES PASS_REGULAR_EXPRESSION "dim\\[\\] and datatype overflow the volume size" ) + + # The NIFTI-1 library carries the same ASCII reader. + add_test( NAME ${TEST_PREFIX}_tool_ascii_dup_attr COMMAND $ -disp_nim -infiles ${CMAKE_CURRENT_LIST_DIR}/testdata/dup_attr.nia ) + set_tests_properties( ${TEST_PREFIX}_tool_ascii_dup_attr PROPERTIES PASS_REGULAR_EXPRESSION "dup2.nia" ) + + if(FSLSTYLE_NAME_CONFLICTS) + add_test( NAME ${TEST_PREFIX}_ambiguous_hdrname + COMMAND ${CMAKE_COMMAND} -DTOOL=$ + -DDIR=${CMAKE_CURRENT_BINARY_DIR}/ambig1 + -P ${CMAKE_CURRENT_LIST_DIR}/ambiguous_hdrname.cmake ) + endif() + unset(TEST_SUFFIX) unset(TEST_PREFIX) unset(TOOL_NAME) diff --git a/niftilib/ambiguous_hdrname.cmake b/niftilib/ambiguous_hdrname.cmake new file mode 100644 index 00000000..3353b6da --- /dev/null +++ b/niftilib/ambiguous_hdrname.cmake @@ -0,0 +1,30 @@ +# Drives the ambiguous-filename path, which must report the clash to its +# caller. A plain add_test cannot tell exit 1 from the abort it replaced. +file(REMOVE_RECURSE ${DIR}) +file(MAKE_DIRECTORY ${DIR}) + +execute_process(COMMAND ${TOOL} -make_im -new_dim 3 4 4 4 0 0 0 0 + -prefix ${DIR}/amb.nii RESULT_VARIABLE mk1) +execute_process(COMMAND ${TOOL} -make_im -new_dim 3 4 4 4 0 0 0 0 + -prefix ${DIR}/amb.nii.gz RESULT_VARIABLE mk2) +if(NOT mk1 STREQUAL "0" OR NOT mk2 STREQUAL "0") + message(FATAL_ERROR "could not create the ambiguous pair: ${mk1} ${mk2}") +endif() + +execute_process(COMMAND ${TOOL} -disp_hdr -infiles ${DIR}/amb + RESULT_VARIABLE rv ERROR_VARIABLE err OUTPUT_VARIABLE out) +if(NOT rv STREQUAL "1") + message(FATAL_ERROR "expected exit 1, got '${rv}'\n${err}") +endif() +if(NOT err MATCHES "Multiple possible filenames") + message(FATAL_ERROR "missing ambiguity diagnostic:\n${err}") +endif() + +# Unambiguous names must still resolve, so that returning NULL +# unconditionally could not pass. +file(REMOVE ${DIR}/amb.nii.gz) +execute_process(COMMAND ${TOOL} -disp_hdr -infiles ${DIR}/amb + RESULT_VARIABLE rv2 ERROR_VARIABLE err2 OUTPUT_VARIABLE out2) +if(NOT rv2 STREQUAL "0") + message(FATAL_ERROR "unambiguous name failed with '${rv2}'\n${err2}") +endif() diff --git a/niftilib/nifti1_io.c b/niftilib/nifti1_io.c index eb8887f3..6eb2f8dd 100644 --- a/niftilib/nifti1_io.c +++ b/niftilib/nifti1_io.c @@ -411,6 +411,9 @@ static const nifti_type_ele nifti_type_list[] = { }; /*---------------------------------------------------------------------------*/ +/* Defined in both libniftiio and libnifti2; linkage pending that duplication. */ +int nifti_fileexists(const char* fname); + /* prototypes for internal functions - not part of exported library */ /* extension routines */ @@ -604,7 +607,7 @@ static void update_nifti_image_for_brick_list( nifti_image * nim , int nbricks ) /* compute nvox */ /* do not rely on dimensions above dim[0] 16 Nov 2005 [rickr] */ for( nim->nvox = 1, ndim = 1; ndim <= nim->dim[0]; ndim++ ) - nim->nvox *= nim->dim[ndim]; + nim->nvox *= (size_t)nim->dim[ndim]; /* update the dimensions to 4 or lower */ for( ndim = 4; (ndim > 1) && (nim->dim[ndim] <= 1); ndim-- ) @@ -698,7 +701,7 @@ int nifti_update_dims_from_array( nifti_image * nim ) nim->dw = nim->pixdim[7]; for( c = 1, nim->nvox = 1; c <= nim->dim[0]; c++ ) - nim->nvox *= nim->dim[c]; + nim->nvox *= (size_t)nim->dim[c]; /* compute ndim, assuming it can be no larger than the old one */ for( ndim = nim->dim[0]; (ndim > 1) && (nim->dim[ndim] <= 1); ndim-- ) @@ -831,7 +834,7 @@ static int nifti_load_NBL_bricks( nifti_image * nim , const int * slist, const i fprintf(stderr,"** load bricks: ztell failed??\n"); return -1; } - fposn = oposn = test; + fposn = oposn = (size_t)test; /* first, handle the default case, no passed blist */ if( !slist ){ @@ -864,8 +867,8 @@ static int nifti_load_NBL_bricks( nifti_image * nim , const int * slist, const i if( isrc != prev ){ /* if we are not looking at the correct sub-brick, scan forward */ - if( fposn != (oposn + isrc*NBL->bsize) ){ - fposn = oposn + isrc*NBL->bsize; + if( fposn != (oposn + (size_t)isrc*NBL->bsize) ){ + fposn = oposn + (size_t)isrc*NBL->bsize; if( znzseek(fp, (long)fposn, SEEK_SET) < 0 ){ fprintf(stderr,"** failed to locate brick %d in file '%s'\n", isrc, nim->iname ? nim->iname : nim->fname); @@ -915,8 +918,9 @@ static int nifti_alloc_NBL_mem(const nifti_image * nim, int nbricks, nbl->nbricks *= nim->dim[c]; } - nbl->bsize = (size_t)nim->nx * nim->ny * nim->nz * nim->nbyper;/* bytes */ - nbl->bricks = (void **)malloc(nbl->nbricks * sizeof(void *)); + nbl->bsize = (size_t)nim->nx * (size_t)nim->ny + * (size_t)nim->nz * (size_t)nim->nbyper; /* bytes */ + nbl->bricks = (void **)malloc((size_t)nbl->nbricks * sizeof(void *)); if( ! nbl->bricks ){ fprintf(stderr,"** NANM: failed to alloc %d void ptrs\n",nbricks); @@ -966,8 +970,8 @@ static int nifti_copynsort(int nbricks, const int * blist, int ** slist, int * stmp, * itmp; /* for ease of typing/reading */ int c1, c2, spos, tmp; - *slist = (int *)malloc(nbricks * sizeof(int)); - *sindex = (int *)malloc(nbricks * sizeof(int)); + *slist = (int *)malloc((size_t)nbricks * sizeof(int)); + *sindex = (int *)malloc((size_t)nbricks * sizeof(int)); if( !*slist || !*sindex ){ fprintf(stderr,"** NCS: failed to alloc %d ints for sorting\n",nbricks); @@ -977,7 +981,7 @@ static int nifti_copynsort(int nbricks, const int * blist, int ** slist, } /* init the lists */ - memcpy(*slist, blist, nbricks*sizeof(int)); + memcpy(*slist, blist, (size_t)nbricks * sizeof(int)); for( c1 = 0; c1 < nbricks; c1++ ) (*sindex)[c1] = c1; /* now actually sort slist */ @@ -1175,13 +1179,13 @@ char *nifti_strdup(const char *str) { if( !str ) return NULL; /* allow calls passing NULL */ - size_t length = strlen(str); - char *dup = (char *)malloc(length + 1); + size_t length = strlen(str) + 1; + char *dup = (char *)malloc(length); /* check for failure */ - if( dup ) strcpy(dup, str); + if( dup ) strlcpy(dup, str, length); else fprintf(stderr,"** nifti_strdup: failed to alloc %zu bytes\n", - length+1); + length); return dup; } @@ -2445,7 +2449,7 @@ int nifti_get_filesize( const char *pathname ) if( pathname == NULL || *pathname == '\0' ) return -1 ; ii = stat( pathname , &buf ); if( ii != 0 ) return -1 ; - return (unsigned int)buf.st_size ; + return (int)buf.st_size ; } #else /*---------- non-Unix version of the above, less efficient -----------*/ @@ -2611,7 +2615,7 @@ const char * nifti_find_file_extension( const char * name ) ext = name + len - 4; /* make manipulation copy, and possibly convert to lowercase */ - strcpy(extcopy, ext); + strlcpy(extcopy, ext, sizeof(extcopy)); if( g_opts.allow_upper_fext ) make_lowercase(extcopy); /* if it look like a basic extension, fail or return it */ @@ -2629,11 +2633,13 @@ const char * nifti_find_file_extension( const char * name ) ext = name + len - 7; /* make manipulation copy, and possibly convert to lowercase */ - strcpy(extcopy, ext); + strlcpy(extcopy, ext, sizeof(extcopy)); if( g_opts.allow_upper_fext ) make_lowercase(extcopy); /* go after .gz extensions using the modifiable strings */ - strcat(elist[0], extgz); strcat(elist[1], extgz); strcat(elist[2], extgz); + strlcat(elist[0], extgz, 8); + strlcat(elist[1], extgz, 8); + strlcat(elist[2], extgz, 8); if( compare_strlist(extcopy, elist, 3) >= 0 ) { if( is_mixedcase(ext) ) { @@ -2808,34 +2814,46 @@ char * nifti_findhdrname(const char* fname) make_uppercase(extzip); } - hdrname = (char *)calloc(sizeof(char),strlen(basename)+8); + size_t hdrnamelength = strlen(basename)+8; + hdrname = (char *)calloc(sizeof(char),hdrnamelength); if( !hdrname ){ fprintf(stderr,"** nifti_findhdrname: failed to alloc hdrname\n"); free(basename); return NULL; } - strcpy(hdrname,basename); - strcat(hdrname,elist[efirst]); + strlcpy(hdrname, basename, hdrnamelength); + strlcat(hdrname, elist[efirst], hdrnamelength); #ifdef FSLSTYLE if (nifti_fileexists(hdrname)) { - free(basename); - char *gzname = (char *)calloc(sizeof(char),strlen(hdrname)+8); - strcpy(gzname, hdrname); - strcat(gzname,extzip); + /* basename is read by the error message below, so it cannot be + freed here; gzname's allocation is used at once, so it has to be + checked; and a library reports an ambiguous name to its caller + rather than ending the process, which means every path out of + here now has to release what it holds. */ + size_t gznamelength = strlen(hdrname)+8; + char *gzname = (char *)calloc(sizeof(char),gznamelength); + if( !gzname ){ + fprintf(stderr,"** nifti_findhdrname: failed to alloc gzname\n"); + free(basename); free(hdrname); + return NULL; + } + strlcpy(gzname, hdrname, gznamelength); + strlcat(gzname,extzip,gznamelength); if (nifti_fileexists(gzname)) { fprintf(stderr,"Image Exception : Multiple possible filenames detected for basename (*.nii, *.nii.gz): %s\n", basename); - free(gzname); - exit(134); + free(gzname); free(basename); free(hdrname); + return NULL; } free(gzname); + free(basename); return hdrname; } #else if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif #ifdef HAVE_ZLIB - strcat(hdrname,extzip); + strlcat(hdrname, extzip, hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif @@ -2843,11 +2861,11 @@ char * nifti_findhdrname(const char* fname) efirst = 1 - efirst; - strcpy(hdrname,basename); - strcat(hdrname,elist[efirst]); + strlcpy(hdrname, basename, hdrnamelength); + strlcat(hdrname, elist[efirst], hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #ifdef HAVE_ZLIB - strcat(hdrname,extzip); + strlcat(hdrname, extzip, hdrnamelength); if (nifti_fileexists(hdrname)) { free(basename); return hdrname; } #endif @@ -2886,8 +2904,9 @@ char * nifti_findimgname(const char* fname , int nifti_type) /* check input file(s) for sanity */ if( !nifti_validfilename(fname) ) return NULL; - basename = nifti_makebasename(fname); - imgname = (char *)calloc(sizeof(char),strlen(basename)+8); + basename = nifti_makebasename(fname); + size_t imgnamelength = strlen(basename)+8; + imgname = (char *)calloc(sizeof(char),imgnamelength); if( !imgname ){ fprintf(stderr,"** nifti_findimgname: failed to alloc imgname\n"); free(basename); @@ -2905,8 +2924,8 @@ char * nifti_findimgname(const char* fname , int nifti_type) /* only valid extension for ASCII type is .nia, handle first */ if( nifti_type == NIFTI_FTYPE_ASCII ){ - strcpy(imgname,basename); - strcat(imgname,extnia); + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, extnia, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } } else { @@ -2919,21 +2938,21 @@ char * nifti_findimgname(const char* fname , int nifti_type) if (nifti_type == NIFTI_FTYPE_NIFTI1_1) first = 0; /* should match .nii */ else first = 1; /* should match .img */ - strcpy(imgname,basename); - strcat(imgname,elist[first]); + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, elist[first], imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #ifdef HAVE_ZLIB /* then also check for .gz */ - strcat(imgname,extzip); + strlcat(imgname, extzip, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #endif /* failed to find image file with expected extension, try the other */ - strcpy(imgname,basename); - strcat(imgname,elist[1-first]); /* can do this with only 2 choices */ + strlcpy(imgname, basename, imgnamelength); + strlcat(imgname, elist[1-first], imgnamelength); /* can do this with only 2 choices */ if (nifti_fileexists(imgname)) { free(basename); return imgname; } #ifdef HAVE_ZLIB /* then also check for .gz */ - strcat(imgname,extzip); + strlcat(imgname, extzip, imgnamelength); if (nifti_fileexists(imgname)) { free(basename); return imgname; } #endif } @@ -2973,9 +2992,10 @@ char * nifti_makehdrname(const char * prefix, int nifti_type, int check, if( !nifti_validfilename(prefix) ) return NULL; /* add space for extension, optional ".gz", and null char */ - iname = (char *)calloc(sizeof(char),strlen(prefix)+8); + size_t inamelength = strlen(prefix)+8; + iname = (char *)calloc(sizeof(char),inamelength); if( !iname ){ fprintf(stderr,"** small malloc failure!\n"); return NULL; } - strcpy(iname, prefix); + strlcpy(iname, prefix, inamelength); /* use any valid extension */ if( (ext = nifti_find_file_extension(iname)) != NULL ){ @@ -2994,12 +3014,12 @@ char * nifti_makehdrname(const char * prefix, int nifti_type, int check, } } /* otherwise, make one up */ - else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_ASCII ) strcat(iname, extnia); - else strcat(iname, exthdr); + else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_ASCII ) strlcat(iname, extnia, inamelength); + else strlcat(iname, exthdr, inamelength); #ifdef HAVE_ZLIB /* if compression is requested, make sure of suffix */ - if( comp && (!ext || !strstr(iname,extgz)) ) strcat(iname,extgz); + if( comp && (!ext || !strstr(iname,extgz)) ) strlcat(iname, extgz, inamelength); #endif /* check for existence failure */ @@ -3043,9 +3063,10 @@ char * nifti_makeimgname(const char * prefix, int nifti_type, int check, if( !nifti_validfilename(prefix) ) return NULL; /* add space for extension, optional ".gz", and null char */ - iname = (char *)calloc(sizeof(char),strlen(prefix)+8); + size_t inamelength = strlen(prefix)+8; + iname = (char *)calloc(sizeof(char),inamelength); if( !iname ){ fprintf(stderr,"** small malloc failure!\n"); return NULL; } - strcpy(iname, prefix); + strlcpy(iname, prefix, inamelength); /* use any valid extension */ if( (ext = nifti_find_file_extension(iname)) != NULL ){ @@ -3064,12 +3085,12 @@ char * nifti_makeimgname(const char * prefix, int nifti_type, int check, } } /* otherwise, make one up */ - else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcat(iname, extnii); - else if( nifti_type == NIFTI_FTYPE_ASCII ) strcat(iname, extnia); - else strcat(iname, extimg); + else if( nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcat(iname, extnii, inamelength); + else if( nifti_type == NIFTI_FTYPE_ASCII ) strlcat(iname, extnia, inamelength); + else strlcat(iname, extimg, inamelength); #ifdef HAVE_ZLIB /* if compression is requested, make sure of suffix */ - if( comp && (!ext || !strstr(iname,extgz)) ) strcat(iname,extgz); + if( comp && (!ext || !strstr(iname,extgz)) ) strlcat(iname, extgz, inamelength); #endif /* check for existence failure */ @@ -3695,7 +3716,7 @@ nifti_image* nifti_convert_nhdr2nim(struct nifti_1_header nhdr, * the qform_code will be zero, at which point you can check * analyze75_orient if you care to. */ - unsigned char c = *((char *)(&nhdr.qform_code)); + unsigned char c = *((unsigned char *)(&nhdr.qform_code)); nim->analyze75_orient = (analyze_75_orient_code)c; } if( doswap ) { @@ -3763,8 +3784,15 @@ nifti_image* nifti_convert_nhdr2nim(struct nifti_1_header nhdr, nim->nv = nim->dim[6] = nhdr.dim[6]; nim->nw = nim->dim[7] = nhdr.dim[7]; - for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ) - nim->nvox *= nhdr.dim[ii]; + /* the product of the dimensions becomes an allocation size, so refuse + the header rather than let it wrap. nvox is a size_t here, where the + NIFTI-2 library uses int64_t, so the bound is SIZE_MAX. */ + for( ii=1, nim->nvox=1; ii <= nhdr.dim[0]; ii++ ){ + if( nhdr.dim[ii] > 0 && nim->nvox > SIZE_MAX / (size_t)nhdr.dim[ii] ){ + free(nim); ERREX("dim[] overflows the voxel count"); + } + nim->nvox *= (size_t)nhdr.dim[ii]; + } /**- set the type of data in voxels and how many bytes per voxel */ @@ -3773,6 +3801,11 @@ nifti_image* nifti_convert_nhdr2nim(struct nifti_1_header nhdr, nifti_datatype_sizes( nim->datatype , &(nim->nbyper) , &(nim->swapsize) ) ; if( nim->nbyper == 0 ){ free(nim); ERREX("bad datatype"); } + /* nifti_get_volsize() multiplies these two */ + if( nim->nvox > SIZE_MAX / (size_t)nim->nbyper ){ + free(nim); ERREX("dim[] and datatype overflow the volume size"); + } + /**- set the grid spacings */ nim->dx = nim->pixdim[1] = nhdr.pixdim[1] ; @@ -4290,13 +4323,6 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) /**- convert all nhdr fields to nifti_image fields */ nim = nifti_convert_nhdr2nim(nhdr,hfile); - #ifdef REJECT_COMPLEX - if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) { - fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname); - exit(13); - } - #endif - if( nim == NULL ){ znzclose( fp ) ; /* close the file */ if( g_opts.debug > 0 ) @@ -4305,14 +4331,21 @@ nifti_image *nifti_image_read( const char *hname , int read_data ) return NULL; } + #ifdef REJECT_COMPLEX + if ((nim->datatype == DT_COMPLEX64) || (nim->datatype == DT_COMPLEX128) || (nim->datatype == DT_COMPLEX256)) { + fprintf(stderr,"Image Exception Unsupported datatype (COMPLEX64): use fslcomplex to manipulate: %s\n", hname); + exit(13); + } + #endif + if( g_opts.debug > 3 ){ fprintf(stderr,"+d nifti_image_read(), have nifti image:\n"); nifti_image_infodump(nim); } /**- check for extensions (any errors here means no extensions) */ - if( NIFTI_ONEFILE(nhdr) ) remaining = nim->iname_offset - sizeof(nhdr); - else remaining = filesize - sizeof(nhdr); + if( NIFTI_ONEFILE(nhdr) ) remaining = nim->iname_offset - (int)sizeof(nhdr); + else remaining = filesize - (int)sizeof(nhdr); (void)nifti_read_extensions(nim, fp, remaining); @@ -4393,12 +4426,12 @@ nifti_image * nifti_read_ascii_image(znzFile fp, char *fname, int flen, fprintf(stderr,"-d %s: have ASCII NIFTI file of size %d\n",fname,slen); if( slen > 65530 ) slen = 65530 ; - sbuf = (char *)calloc(sizeof(char),slen+1) ; + sbuf = (char *)calloc(sizeof(char), (size_t)(slen+1)) ; if( !sbuf ){ fprintf(stderr,"** %s: failed to alloc %d bytes for sbuf",lfunc,65530); return NULL; } - znzread( sbuf , 1 , slen , fp ) ; + znzread( sbuf , 1 , (size_t)slen, fp ) ; nim = nifti_image_from_ascii( sbuf, &txt_size ) ; free( sbuf ) ; if( nim == NULL ){ LNI_FERR(lfunc,"failed nifti_image_from_ascii()",fname); @@ -4582,12 +4615,12 @@ static int nifti_add_exten_to_list( nifti1_extension * new_ext, nifti1_extension * tmplist; tmplist = *list; - *list = (nifti1_extension *)malloc(new_length * sizeof(nifti1_extension)); + *list = (nifti1_extension *)malloc((size_t)new_length * sizeof(nifti1_extension)); /* check for failure first */ if( ! *list ){ fprintf(stderr,"** failed to alloc %d extension structs (%zu bytes)\n", - new_length, new_length*sizeof(nifti1_extension)); + new_length, (size_t)new_length * sizeof(nifti1_extension)); if( !tmplist ) return -1; /* no old list to lose */ *list = tmplist; /* reset list to old one */ @@ -4596,7 +4629,7 @@ static int nifti_add_exten_to_list( nifti1_extension * new_ext, /* if an old list exists, copy the pointers and free the list */ if( tmplist ){ - memcpy(*list, tmplist, (new_length-1)*sizeof(nifti1_extension)); + memcpy(*list, tmplist, (size_t)(new_length-1)*sizeof(nifti1_extension)); free(tmplist); } @@ -4640,13 +4673,13 @@ static int nifti_fill_extension( nifti1_extension *ext, const char * data, ext->esize = esize; /* allocate esize-8 (maybe more than len), using calloc for fill */ - ext->edata = (char *)calloc(esize-8, sizeof(char)); + ext->edata = (char *)calloc((size_t)(esize-8),sizeof(char)); if( !ext->edata ){ fprintf(stderr,"** NFE: failed to alloc %d bytes for extension\n",len); return -1; } - memcpy(ext->edata, data, len); /* copy the data, using len */ + memcpy(ext->edata, data, (size_t)len); /* copy the data, using len */ ext->ecode = ecode; /* set the ecode */ if( g_opts.debug > 2 ) @@ -4689,7 +4722,7 @@ static int nifti_read_next_extension( nifti1_extension * nex, nifti_image *nim, if( count != 2 ){ if( g_opts.debug > 2 ) fprintf(stderr,"-d current extension read failed\n"); - znzseek(fp, -4*count, SEEK_CUR); /* back up past any read */ + znzseek(fp, -4L*count, SEEK_CUR); /* back up past any read */ return 0; /* no extension, no error condition */ } @@ -4717,13 +4750,13 @@ static int nifti_read_next_extension( nifti1_extension * nex, nifti_image *nim, nex->ecode = code; size -= 8; /* subtract space for size and code in extension */ - nex->edata = (char *)malloc(size * sizeof(char)); + nex->edata = (char *)malloc((size_t)size * sizeof(char)); if( !nex->edata ){ fprintf(stderr,"** failed to allocate %d bytes for extension\n",size); return -1; } - count = (int)znzread(nex->edata, 1, size, fp); + count = (int)znzread(nex->edata, 1, (size_t)size, fp); if( count < size ){ if( g_opts.debug > 0 ) fprintf(stderr,"-d read only %d (of %d) bytes for extension\n", @@ -4907,15 +4940,17 @@ static znzFile nifti_image_load_prep( nifti_image *nim ) znzclose(fp); return NULL; } - ii = nifti_get_filesize( nim->iname ) ; - if( ii == 0 ){ - if( g_opts.debug > 0 ) LNI_FERR(fname,"empty data file",nim->iname); + const int64_t fsize = nifti_get_filesize( nim->iname ) ; + if( fsize <= 0 ){ + if( g_opts.debug > 0 ) + LNI_FERR(fname,"empty or unreadable data file",nim->iname); znzclose(fp); return NULL; } + ii = (size_t)fsize ; ioff = (ii > ntot) ? ii-ntot : 0 ; } else { /* non-negative offset */ - ioff = nim->iname_offset ; /* means use it directly */ + ioff = (size_t)nim->iname_offset ; /* means use it directly */ } /**- seek to the appropriate read position */ @@ -5019,7 +5054,7 @@ size_t nifti_read_buffer(znzFile fp, void* dataptr, size_t ntot, if( dataptr == NULL ){ if( g_opts.debug > 0 ) fprintf(stderr,"** ERROR: nifti_read_buffer: NULL dataptr\n"); - return -1; + return 0; } ii = znzread( dataptr , 1 , ntot , fp ) ; /* data input */ @@ -5034,7 +5069,7 @@ size_t nifti_read_buffer(znzFile fp, void* dataptr, size_t ntot, nim->iname , (unsigned int)ntot , (unsigned int)ii , (unsigned int)(ntot-ii) ) ; /* memset( (char *)(dataptr)+ii , 0 , ntot-ii ) ; now failure [rickr] */ - return -1 ; + return 0 ; } if( g_opts.debug > 2 ) @@ -5046,7 +5081,7 @@ size_t nifti_read_buffer(znzFile fp, void* dataptr, size_t ntot, if( nim->swapsize > 1 && nim->byteorder != nifti_short_order() ) { if( g_opts.debug > 1 ) fprintf(stderr,"+d nifti_read_buffer: swapping data bytes...\n"); - nifti_swap_Nbytes( (int)(ntot / nim->swapsize), nim->swapsize , dataptr ) ; + nifti_swap_Nbytes( ntot / (size_t)nim->swapsize, nim->swapsize , dataptr ) ; } #ifdef isfinite @@ -5222,11 +5257,11 @@ int nifti_write_all_data(znzFile fp, nifti_image * nim, return -1; } - ss = nifti_write_buffer(fp,nim->data,nim->nbyper * nim->nvox); - if (ss < nim->nbyper * nim->nvox){ + ss = nifti_write_buffer(fp,nim->data, (size_t)nim->nbyper * nim->nvox); + if (ss < (size_t)nim->nbyper * nim->nvox){ fprintf(stderr, "** ERROR: NWAD: wrote only %u of %u bytes to file\n", - (unsigned)ss, (unsigned)(nim->nbyper * nim->nvox)); + (unsigned)ss, (unsigned)((size_t)nim->nbyper * nim->nvox)); return -1; } @@ -5299,7 +5334,7 @@ static int nifti_write_extensions(znzFile fp, nifti_image *nim) ok = (size == (int)sizeof(int)); } if( ok ){ - size = (int)nifti_write_buffer(fp, list->edata, list->esize - 8); + size = (int)nifti_write_buffer(fp, list->edata, (size_t)(list->esize - 8)); ok = (size == list->esize - 8); } @@ -5345,7 +5380,7 @@ nifti_image* nifti_simple_init_nim(void) nifti_datatype_sizes( nhdr.datatype , &nbyper, &swapsize ); nhdr.bitpix = 8 * nbyper ; - strcpy(nhdr.magic, "n+1"); /* init to single file */ + strlcpy(nhdr.magic, "n+1", sizeof(nhdr.magic)); /* init to single file */ nim = nifti_convert_nhdr2nim(nhdr,NULL); nim->fname = NULL; @@ -5426,7 +5461,7 @@ nifti_1_header * nifti_make_new_header(const int arg_dims[8], int arg_dtype) nifti_datatype_sizes( nhdr->datatype , &nbyper, &swapsize ); nhdr->bitpix = 8 * nbyper ; - strcpy(nhdr->magic, "n+1"); /* init to single file */ + strlcpy(nhdr->magic, "n+1", sizeof(nhdr->magic)); /* init to single file */ return nhdr; } @@ -5463,12 +5498,12 @@ nifti_image * nifti_make_new_nim(const int dims[8], int datatype, int data_fill) fprintf(stderr,"+d nifti_make_new_nim, data_fill = %d\n",data_fill); if( data_fill ) { - nim->data = calloc(nim->nvox, nim->nbyper); + nim->data = calloc(nim->nvox, (size_t)(nim->nbyper)); /* if we cannot allocate data, take ball and go home */ if( !nim->data ) { fprintf(stderr,"** NMNN: failed to alloc %u bytes for data\n", - (unsigned)(nim->nvox*nim->nbyper)); + (unsigned)(nim->nvox*(size_t)nim->nbyper)); nifti_image_free(nim); nim = NULL; } @@ -5535,8 +5570,8 @@ struct nifti_1_header nifti_convert_nim2nhdr(const nifti_image * nim) if( nim->nifti_type > NIFTI_FTYPE_ANALYZE ){ /* then not ANALYZE */ - if( nim->nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strcpy(nhdr.magic,"n+1") ; - else strcpy(nhdr.magic,"ni1") ; + if( nim->nifti_type == NIFTI_FTYPE_NIFTI1_1 ) strlcpy(nhdr.magic, "n+1", sizeof(nhdr.magic)) ; + else strlcpy(nhdr.magic, "ni1", sizeof(nhdr.magic)) ; nhdr.pixdim[1] = (float)fabs(nhdr.pixdim[1]) ; nhdr.pixdim[2] = (float)fabs(nhdr.pixdim[2]) ; nhdr.pixdim[3] = (float)fabs(nhdr.pixdim[3]) ; nhdr.pixdim[4] = (float)fabs(nhdr.pixdim[4]) ; @@ -5625,7 +5660,7 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) if( nim_src->num_ext <= 0 ) return 0; - bytes = nim_src->num_ext * sizeof(nifti1_extension); /* I'm lazy */ + bytes = (size_t)nim_src->num_ext * sizeof(nifti1_extension); /* I'm lazy */ nim_dest->ext_list = (nifti1_extension *)malloc(bytes); if( !nim_dest->ext_list ){ fprintf(stderr,"** failed to allocate %d nifti1_extension structs\n", @@ -5642,7 +5677,7 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) fprintf(stderr,"+d dup'ing ext #%d of size %d (from size %d)\n", c, size, old_size); /* data length is size-8, as esize includes space for esize and ecode */ - data = (char *)calloc(size-8,sizeof(char)); /* maybe size > old */ + data = (char *)calloc((size_t)(size-8),sizeof(char)); /* maybe size > old */ if( !data ){ fprintf(stderr,"** failed to alloc %d bytes for extension\n", size); if( c == 0 ) { free(nim_dest->ext_list); nim_dest->ext_list = NULL; } @@ -5653,7 +5688,7 @@ int nifti_copy_extensions(nifti_image * nim_dest, const nifti_image * nim_src) nim_dest->ext_list[c].esize = size; nim_dest->ext_list[c].ecode = nim_src->ext_list[c].ecode; nim_dest->ext_list[c].edata = data; - memcpy(data, nim_src->ext_list[c].edata, old_size-8); + memcpy(data, nim_src->ext_list[c].edata, (size_t)(old_size-8)); nim_dest->num_ext++; } @@ -5711,7 +5746,7 @@ void nifti_set_iname_offset(nifti_image *nim) /* NIFTI-1 single binary file - always update */ case NIFTI_FTYPE_NIFTI1_1: - offset = nifti_extension_size(nim)+sizeof(struct nifti_1_header)+4; + offset = nifti_extension_size(nim)+(int)sizeof(struct nifti_1_header)+4; /* be sure offset is aligned to a 16 byte boundary */ if ( ( offset % 16 ) != 0 ) offset = ((offset + 0xf) & ~0xf); if( nim->iname_offset != offset ){ @@ -5773,13 +5808,14 @@ znzFile nifti_image_write_hdr_img2(nifti_image *nim, int write_opts, #ifdef PIGZ #ifdef HAVE_ZLIB +#if 0 /* unused here: identical to doPigz below */ int doPigz2(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) { FILE *pigzPipe; char command[768]; - strcpy(command, "pigz" ); - strcat(command, " -n -f > \""); - strcat(command, nim->fname); - strcat(command, "\""); + strlcpy(command, "pigz", sizeof(command)); + strlcat(command, " -n -f > \"", sizeof(command)); + strlcat(command, nim->fname, sizeof(command)); + strlcat(command, "\"", sizeof(command)); #ifdef _MSC_VER if (( pigzPipe = _popen(command, "w")) == NULL) return -1; @@ -5804,14 +5840,15 @@ int doPigz2(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list free(fp); return 0; } +#endif -int doPigz(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) { +static int doPigz(nifti_image *nim, struct nifti_1_header nhdr, const nifti_brick_list * NBL) { FILE *pigzPipe; char command[768]; - strcpy(command, "pigz" ); - strcat(command, " -n -f > \""); - strcat(command, nim->fname); - strcat(command, "\""); + strlcpy(command, "pigz", sizeof(command)); + strlcat(command, " -n -f > \"", sizeof(command)); + strlcat(command, nim->fname, sizeof(command)); + strlcat(command, "\"", sizeof(command)); #ifdef _MSC_VER if (( pigzPipe = _popen(command, "w")) == NULL) return -1; @@ -6295,7 +6332,7 @@ static char *escapize_string( const char * str ) default: lout++ ; break ; /* copy all other chars */ } } - out = (char *)calloc(1,lout) ; /* allocate output string */ + out = (char *)calloc(1, (size_t)lout) ; /* allocate output string */ if( !out ){ fprintf(stderr,"** escapize_string: failed to alloc %d bytes\n",lout); return NULL; @@ -6569,7 +6606,7 @@ char *nifti_image_to_ascii( const nifti_image *nim ) snprintf( buf+strlen(buf) , bufLen-strlen(buf) , "/>\n" ) ; /* XML-ish closer */ nbuf = (int)strlen(buf) ; - newbuf = (char *)realloc((void *)buf, nbuf+1); /* cut back to proper length */ + newbuf = (char *)realloc((void *)buf, (size_t)(nbuf+1)); /* cut back to proper length */ if( !newbuf ){ free(buf); fprintf(stderr,"** NITA: failed to realloc %d bytes\n",nbuf+1); @@ -6614,7 +6651,7 @@ int nifti_short_order(void) /* determine this CPU's byte order */ put rhs string into nim->"nam" string, with field size = "sz" */ #define QSTR(nam,sz) if( strcmp(lhs,#nam) == 0 ) \ - strncpy(nim->nam,rhs,sz), nim->nam[sz-1]='\0' + memset(nim->nam, 0, sz), strlcpy(nim->nam,rhs,sz) /*---------------------------------------------------------------------------*/ /*! Take an XML-ish ASCII string and create a NIFTI image header to match. @@ -6681,7 +6718,7 @@ nifti_image *nifti_image_from_ascii( const char *str, int * bytes_read ) ii = spos+1 ; while( str[ii] != '\0' && str[ii] != '\'' ) ii++ ; nn = ii-spos-1 ; if( nn > 1023 ) nn = 1023 ; - memcpy(rhs,str+spos+1,nn) ; rhs[nn] = '\0' ; + memcpy(rhs,str+spos+1, (size_t)nn) ; rhs[nn] = '\0' ; spos = (str[ii] == '\'') ? ii+1 : ii ; } else { ii = sscanf( str+spos , "%1023s%n" , rhs , &nn ) ; spos += nn ; @@ -6703,9 +6740,11 @@ nifti_image *nifti_image_from_ascii( const char *str, int * bytes_read ) nim->nifti_type = NIFTI_FTYPE_ASCII ; } else if( strcmp(lhs,"header_filename") == 0 ){ + free(nim->fname) ; /* the attribute may appear more than once */ nim->fname = nifti_strdup(rhs) ; } else if( strcmp(lhs,"image_filename") == 0 ){ + free(nim->iname) ; nim->iname = nifti_strdup(rhs) ; } else if( strcmp(lhs,"sto_xyz_matrix") == 0 ){ @@ -6792,8 +6831,9 @@ nifti_image *nifti_image_from_ascii( const char *str, int * bytes_read ) nim->dim[6] = nim->nv ; nim->pixdim[6] = nim->dv ; nim->dim[7] = nim->nw ; nim->pixdim[7] = nim->dw ; - nim->nvox = (size_t)nim->nx * nim->ny * nim->nz - * nim->nt * nim->nu * nim->nv * nim->nw ; + nim->nvox = (size_t)nim->nx * (size_t)nim->ny * (size_t)nim->nz + * (size_t)nim->nt * (size_t)nim->nu * (size_t)nim->nv + * (size_t)nim->nw ; if( nim->qform_code > 0 ) nim->qto_xyz = nifti_quatern_to_mat44( @@ -6903,7 +6943,7 @@ int nifti_nim_has_valid_dims(nifti_image * nim, int complain) prod = 1; for( c = 1; c <= nim->dim[0]; c++ ){ if( nim->dim[c] > 0) - prod *= nim->dim[c]; + prod *= (size_t)nim->dim[c]; else { if( !complain ) return 0; fprintf(stderr,"** NVd: dim[%d] (=%d) <= 0\n",c, nim->dim[c]); @@ -7055,7 +7095,7 @@ int nifti_read_collapsed_image( nifti_image * nim, const int dims [8], /** - call the recursive reading function, passing nim, the pivot info, location to store memory, and file pointer and position */ c = rci_read_data(nim, pivots,prods,nprods,dims, - (char *)*data, fp, znztell(fp)); + (char *)*data, fp, (size_t)znztell(fp)); znzclose(fp); /* in any case, close the file */ if( c < 0 ){ free(*data); *data = NULL; return -1; } /* failure */ @@ -7203,12 +7243,12 @@ int nifti_read_subregion_image( nifti_image * nim, /* get strides*/ compute_strides(strides,image_size,nim->nbyper); - total_alloc_size = nim->nbyper; /* size of pixel */ + total_alloc_size = (size_t)nim->nbyper; /* size of pixel */ /* find alloc size */ for(i = 0; i < nim->ndim; i++) { - total_alloc_size *= region_size[i]; + total_alloc_size *= (size_t)region_size[i]; } /* allocate buffer, if necessary */ if(*data == 0) @@ -7267,7 +7307,7 @@ int nifti_read_subregion_image( nifti_image * nim, (si[0] * strides[0]); znzseek(fp, offset, SEEK_SET); /* seek to current row */ read_amount = rs[0] * nim->nbyper; /* read a row of the subregion*/ - nread = (int)nifti_read_buffer(fp, readptr, read_amount, nim); + nread = (int)nifti_read_buffer(fp, readptr, (size_t)read_amount, nim); if(nread != read_amount) { if(g_opts.debug > 0) @@ -7322,7 +7362,7 @@ static int rci_read_data(nifti_image * nim, int * pivots, int * prods, /* so just seek and read (prods[0] * nbyper) bytes from the file */ znzseek(fp, (long)base_offset, SEEK_SET); - bytes = (size_t)prods[0] * nim->nbyper; + bytes = (size_t)prods[0] * (size_t)nim->nbyper; nread = nifti_read_buffer(fp, data, bytes, nim); if( nread != bytes ){ fprintf(stderr,"** rciRD: read only %u of %u bytes from '%s'\n", @@ -7338,29 +7378,29 @@ static int rci_read_data(nifti_image * nim, int * pivots, int * prods, /* not the base case, so do a set of reduced reads */ /* compute size of sub-brick: all dimensions below pivot */ - for( c = 1, sublen = 1; c < *pivots; c++ ) sublen *= nim->dim[c]; + for( c = 1, sublen = 1; c < *pivots; c++ ) sublen *= (size_t)nim->dim[c]; /* compute number of values to read, i.e. remaining prods */ - for( c = 1, read_size = 1; c < nprods; c++ ) read_size *= prods[c]; - read_size *= nim->nbyper; /* and multiply by bytes per voxel */ + for( c = 1, read_size = 1; c < nprods; c++ ) read_size *= (size_t)prods[c]; + read_size *= (size_t)nim->nbyper; /* and multiply by bytes per voxel */ /* now repeatedly compute offsets, and recursively read */ for( c = 0; c < prods[0]; c++ ){ /* offset is (c * sub-block size (including pivot dim)) */ /* + (dims[] index into pivot sub-block) */ /* the unneeded multiplication is to make this more clear */ - offset = (size_t)c * sublen * nim->dim[*pivots] + - (size_t)sublen * dims[*pivots]; - offset *= nim->nbyper; + offset = (size_t)c * sublen * (size_t)nim->dim[*pivots] + + sublen * (size_t)dims[*pivots]; + offset *= (size_t)nim->nbyper; if( g_opts.debug > 3 ) fprintf(stderr,"-d reading %u bytes, foff %u + %u, doff %u\n", (unsigned)read_size, (unsigned)base_offset, (unsigned)offset, - (unsigned)(c*read_size)); + (unsigned)((size_t)c*read_size)); /* now read the next level down, adding this offset */ if( rci_read_data(nim, pivots+1, prods+1, nprods-1, dims, - data + c * read_size, fp, base_offset + offset) < 0 ) + data + (size_t)c * read_size, fp, base_offset + offset) < 0 ) return -1; } @@ -7394,7 +7434,7 @@ static int rci_alloc_mem(void ** data, const int prods[8], int nprods, int nbype fprintf(stderr,"+d alloc %d (= %d x %d) bytes for collapsed image\n", size, size/nbyper, nbyper); - *data = malloc(size); /* actually allocate the memory */ + *data = malloc((size_t)size); /* actually allocate the memory */ if( ! *data ){ fprintf(stderr,"** rci_am: failed to alloc %d bytes for data\n", size); return -1; @@ -7557,7 +7597,7 @@ int * nifti_get_intlist( int nvals , const char * str ) if( str[ipos] == ',' || ISEND(str[ipos]) ){ nout++ ; - subv_realloc = (int *)realloc( (char *)subv , sizeof(int) * (nout+1) ) ; + subv_realloc = (int *)realloc( (char *)subv , sizeof(int) * (size_t)(nout+1)) ; if( !subv_realloc ) { free(subv); fprintf(stderr,"** nifti_get_intlist: failed realloc of %d ints\n", @@ -7648,7 +7688,7 @@ int * nifti_get_intlist( int nvals , const char * str ) for( ii=ibot ; (ii-itop)*istep <= 0 ; ii += istep ){ nout++ ; - subv_realloc = (int *)realloc( (char *)subv , sizeof(int) * (nout+1) ) ; + subv_realloc = (int *)realloc( (char *)subv , sizeof(int) * (size_t)(nout+1)) ; if( !subv_realloc ) { free(subv); fprintf(stderr,"** nifti_get_intlist: failed realloc of %d ints\n", diff --git a/niftilib/nifti1_test.c b/niftilib/nifti1_test.c index 46808353..00fd9bd3 100644 --- a/niftilib/nifti1_test.c +++ b/niftilib/nifti1_test.c @@ -19,7 +19,6 @@ int main( int argc , const char *argv[] ) nifti_image *nim ; int iarg=1 , outmode=1 , argn, usegzip=0; char *tmpstr; - size_t ll; if( argc < 2 || strcmp(argv[1],"-help") == 0 ){ printf("Usage: nifti1_test [-n2|-n1|-na|-a2] infile [prefix]\n" @@ -79,24 +78,24 @@ int main( int argc , const char *argv[] ) free(nim->fname) ; free(nim->iname) ; - ll = strlen(argv[iarg]) ; + size_t ll = strlen(argv[iarg]) + 8 ; tmpstr = nifti_makebasename(argv[iarg]); - nim->fname = (char *)calloc(1,ll+8) ; strcpy(nim->fname,tmpstr) ; - nim->iname = (char *)calloc(1,ll+8) ; strcpy(nim->iname,tmpstr) ; + nim->fname = (char *)calloc(1,ll) ; strlcpy(nim->fname, tmpstr, ll) ; + nim->iname = (char *)calloc(1,ll) ; strlcpy(nim->iname, tmpstr, ll) ; free(tmpstr); if( nim->nifti_type == 1 ){ - strcat(nim->fname,".nii") ; - strcat(nim->iname,".nii") ; + strlcat(nim->fname, ".nii", ll) ; + strlcat(nim->iname, ".nii", ll) ; } else if ( nim->nifti_type == 3 ){ - strcat(nim->fname,".nia") ; - strcat(nim->iname,".nia") ; + strlcat(nim->fname, ".nia", ll) ; + strlcat(nim->iname, ".nia", ll) ; } else { - strcat(nim->fname,".hdr") ; - strcat(nim->iname,".img") ; + strlcat(nim->fname, ".hdr", ll) ; + strlcat(nim->iname, ".img", ll) ; } if (usegzip) { - strcat(nim->fname,".gz"); - strcat(nim->iname,".gz"); + strlcat(nim->fname, ".gz", ll); + strlcat(nim->iname, ".gz", ll); } if( nifti_image_write_status( nim ) ) { fprintf(stderr, "** failed to write nifti_image\n"); diff --git a/niftilib/nifti1_tool.c b/niftilib/nifti1_tool.c index 15bb165a..8ae791df 100644 --- a/niftilib/nifti1_tool.c +++ b/niftilib/nifti1_tool.c @@ -702,7 +702,7 @@ int fill_cmd_string( nt_opts * opts, int argc, const char * argv[]) return 1; } cp = opts->command + len; - remain -= len; + remain -= (size_t)len; /* get the rest, with special attention to input files */ for( int ac = 1; ac < argc; ac++ ) @@ -723,11 +723,11 @@ int fill_cmd_string( nt_opts * opts, int argc, const char * argv[]) if( has_space ) len = snprintf(cp, remain, " '%s'", argv[ac]); else len = snprintf(cp, remain, " %s", argv[ac]); - if( len < 0 || len >= remain ) { + if( len < 0 || (size_t)len >= remain ) { fprintf(stderr,"FCS: error parsing command, continuing...\n"); return 1; } - remain -= len; + remain -= (size_t)len; /* infiles is okay, but after the *next* argument, we may skip files */ /* (danger, will robinson! hack alert!) */ @@ -760,7 +760,7 @@ int add_int(int_list * ilist, int val) { if( ilist->len == 0 ) ilist->list = NULL; /* just to be safe */ ilist->len++; - ilist->list = (int *)realloc(ilist->list,ilist->len*sizeof(int)); + ilist->list = (int *)realloc(ilist->list, (size_t)ilist->len * sizeof(int)); if( ! ilist->list ){ fprintf(stderr,"** failed to alloc %d (int *) elements\n",ilist->len); return -1; @@ -781,7 +781,7 @@ int add_string(str_list * slist, const char * str) { if( slist->len == 0 ) slist->list = NULL; /* just to be safe */ slist->len++; - slist->list = (const char **)realloc(slist->list,slist->len*sizeof(char *)); + slist->list = (const char **)realloc(slist->list, (size_t)slist->len * sizeof(char *)); if( ! slist->list ){ fprintf(stderr,"** failed to alloc %d (char *) elements\n",slist->len); return -1; @@ -1865,6 +1865,7 @@ int act_add_exts( nt_opts * opts ) } if( nifti_add_extension(nim, ext, elen, opts->etypes.list[ec]) ){ + free(edata); /* may hold the file contents read just above */ nifti_image_free(nim); return 1; } @@ -1935,14 +1936,14 @@ static char * read_file_text(const char * filename, int * length) /* allocate the bytes, and fill them with the file contents */ - text = (char *)malloc(len * sizeof(char)); + text = (char *)malloc((size_t)len * sizeof(char)); if( !text ) { fprintf(stderr,"** RFT: failed to allocate %d bytes\n", len); fclose(fp); return NULL; } - bytes = fread(text, sizeof(char), len, fp); + bytes = fread(text, sizeof(char), (size_t)len, fp); fclose(fp); /* in any case */ if( bytes != (size_t)len ) { @@ -2140,7 +2141,7 @@ int remove_ext_list( nifti_image * nim, const char ** elist, int len ) if( g_debug > 2 ) fprintf(stderr,"+d removing %d exts from '%s'\n", len, nim->fname ); - if( ! (marks = (int *)calloc(nim->num_ext, sizeof(int))) ) { + if( ! (marks = (int *)calloc((size_t)(nim->num_ext),sizeof(int))) ) { fprintf(stderr,"** failed to alloc %d marks\n",nim->num_ext); return -1; } @@ -2277,7 +2278,7 @@ int act_diff_nims( nt_opts * opts ) if( ! nim0 ) return 1; /* errors have been printed */ nim1 = nt_image_read(opts, opts->infiles.list[1], 0); - if( ! nim1 ){ free(nim0); return 1; } + if( ! nim1 ){ nifti_image_free(nim0); return 1; } if( g_debug > 1 ) fprintf(stderr,"\n-d nifti_image diffs between '%s' and '%s'...\n", @@ -2606,6 +2607,7 @@ int act_mod_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -2615,6 +2617,7 @@ int act_mod_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -2623,6 +2626,8 @@ int act_mod_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -2727,6 +2732,7 @@ int act_swap_hdrs( nt_opts * opts ) if( !nim ) { fprintf(stderr,"** failed to dup file '%s' before modifying\n", fname); + free(nhdr); return 1; } if( opts->keep_hist && nifti_add_extension(nim, opts->command, @@ -2736,6 +2742,7 @@ int act_swap_hdrs( nt_opts * opts ) { NTL_FERR(func,"failed to set prefix for new file: ",opts->prefix); nifti_image_free(nim); + free(nhdr); return 1; } dupname = nifti_strdup(nim->fname); /* so we know to free it */ @@ -2744,6 +2751,8 @@ int act_swap_hdrs( nt_opts * opts ) if( nifti_image_write_status(nim) ) { fprintf(stderr,"** failed to write image %s\n", nim->fname); nifti_image_free(nim); + free(dupname); + free(nhdr); return 1; } @@ -3022,10 +3031,10 @@ int modify_field(void * basep, field_s * field, const char * data) case NT_DT_STRING: { char * dest = (char *)basep + field->offset; - nchars = dataLength; - strncpy(dest, data, field->len); + nchars = (int)dataLength; + strncpy(dest, data, (size_t)(field->len)); if( nchars < field->len ) /* clear the rest */ - memset(dest+nchars, '\0', field->len-nchars); + memset(dest+nchars, '\0', (size_t)(field->len-nchars)); } break; } @@ -3347,8 +3356,7 @@ int fill_field( field_s * fp, int type, int offset, int num, const char * name ) fp->size = 1; /* init before check */ fp->len = num; - strncpy(fp->name, name, sizeof(fp->name)); - fp->name[sizeof(fp->name) - 1] = 0; + strlcpy(fp->name, name, sizeof(fp->name)); switch( type ){ case DT_UNKNOWN: @@ -4261,8 +4269,9 @@ nifti_image * nt_read_bricks(nt_opts * opts, const char * fname, int len, int * /* now populate NBL (can be based only on len and nim) */ NBL->nbricks = len; - NBL->bsize = (size_t)nim->nbyper * nim->nx * nim->ny * nim->nz; - NBL->bricks = (void **)calloc(NBL->nbricks, sizeof(void *)); + NBL->bsize = (size_t)nim->nbyper * (size_t)nim->nx + * (size_t)nim->ny * (size_t)nim->nz; + NBL->bricks = (void **)calloc((size_t)(NBL->nbricks), (size_t)(sizeof(void *))); if( !NBL->bricks ){ fprintf(stderr,"** NRB: failed to alloc %d pointers\n",NBL->nbricks); nifti_image_free(nim); diff --git a/niftilib/nifti1_tool.h b/niftilib/nifti1_tool.h index a8ca5ddd..b099924a 100644 --- a/niftilib/nifti1_tool.h +++ b/niftilib/nifti1_tool.h @@ -72,10 +72,10 @@ typedef struct{ #define NT_HDR_NUM_FIELDS 43 /* in the nifti_1_header struct */ #define NT_ANA_NUM_FIELDS 47 /* in the nifti_analyze75 struct */ #define NT_NIM_NUM_FIELDS 63 /* in the nifti_image struct */ -#define NT_DT_STRING -0xfff /* some strange number to abuse... */ -#define NT_DT_POINTER -0xfef /* some strange number to abuse... */ -#define NT_DT_CHAR_PTR -0xfee /* another... */ -#define NT_DT_EXT_PTR -0xfed /* and another... */ +#define NT_DT_STRING (-0xfff) /* some strange number to abuse... */ +#define NT_DT_POINTER (-0xfef) /* some strange number to abuse... */ +#define NT_DT_CHAR_PTR (-0xfee) /* another... */ +#define NT_DT_EXT_PTR (-0xfed) /* and another... */ typedef struct { int type; /* one of the DT_* types from nifti1.h */ @@ -157,4 +157,4 @@ nifti_1_header * nt_read_header(nt_opts * opts, const char * fname, int * swappe int check); -#endif /* NIFTI1_TOOL_H */ \ No newline at end of file +#endif /* NIFTI1_TOOL_H */ diff --git a/niftilib/nifti_short_read_test.c b/niftilib/nifti_short_read_test.c new file mode 100644 index 00000000..809fb684 --- /dev/null +++ b/niftilib/nifti_short_read_test.c @@ -0,0 +1,89 @@ +/* A .nii whose data section is short of the header's declared size must be + rejected, not handed back with uninitialized tail bytes. */ + +#include +#include +#include "nifti1_io.h" + +static const char *WHOLE = "short_read_whole.nii"; +static const char *SHORT = "short_read_short.nii"; +static const long MISSING = 1000; + +static int write_whole(void) +{ + int dims[8] = { 3, 31, 31, 31, 1, 1, 1, 1 }; + nifti_image *nim = nifti_make_new_nim(dims, DT_FLOAT32, 1); + if( nim == NULL ) return 1; + if( nifti_set_filenames(nim, WHOLE, 0, 1) != 0 ){ + nifti_image_free(nim); + return 1; + } + nifti_image_write(nim); + nifti_image_free(nim); + return 0; +} + +static int copy_truncated(void) +{ + FILE *in = fopen(WHOLE, "rb"), *out; + long size; + char *buf; + size_t got; + + if( in == NULL ) return 1; + fseek(in, 0, SEEK_END); + size = ftell(in); + fseek(in, 0, SEEK_SET); + if( size <= MISSING ){ fclose(in); return 1; } + + buf = (char *)malloc((size_t)size); + if( buf == NULL ){ fclose(in); return 1; } + got = fread(buf, 1, (size_t)size, in); + fclose(in); + if( got != (size_t)size ){ free(buf); return 1; } + + out = fopen(SHORT, "wb"); + if( out == NULL ){ free(buf); return 1; } + fwrite(buf, 1, (size_t)(size - MISSING), out); + fclose(out); + free(buf); + return 0; +} + +int main(void) +{ + nifti_image *nim; + + if( write_whole() ){ + fprintf(stderr, "FAILURE: could not write the reference image\n"); + return 1; + } + if( copy_truncated() ){ + fprintf(stderr, "FAILURE: could not write the truncated copy\n"); + return 1; + } + + nim = nifti_image_read(SHORT, 1); + if( nim != NULL ){ + fprintf(stderr, "FAILURE: truncated image was accepted, nvox=%d\n", + (int)nim->nvox); + nifti_image_free(nim); + return 1; + } + + /* the rejection must be specific to the short read */ + nim = nifti_image_read(WHOLE, 1); + if( nim == NULL ){ + fprintf(stderr, "FAILURE: the whole image was rejected\n"); + return 1; + } + if( nim->data == NULL ){ + fprintf(stderr, "FAILURE: the whole image came back with no data\n"); + nifti_image_free(nim); + return 1; + } + nifti_image_free(nim); + + printf("Short-read rejection test passed.\n"); + return 0; +} diff --git a/niftilib/nifti_tester001.c b/niftilib/nifti_tester001.c index 3fbc0656..137e7e4e 100644 --- a/niftilib/nifti_tester001.c +++ b/niftilib/nifti_tester001.c @@ -80,11 +80,9 @@ static nifti_image * generate_reference_image( const char * write_image_filename reference_header.magic[1]='+'; reference_header.magic[2]='1'; reference_header.magic[3]='\0'; - /* String is purposfully too long */ - strncpy(reference_header.intent_name,"PHANTOM_DATA to be used for regression testing the nifti reader/writer",sizeof(reference_header.intent_name)); - reference_header.intent_name[sizeof(reference_header.intent_name) - 1] = 0; - strncpy(reference_header.descrip,"This is a very long dialog here to use up more than 80 characters of space to test to see if the code is robust enough to deal appropriately with very long and obnoxious lines.",sizeof(reference_header.descrip)); - reference_header.descrip[sizeof(reference_header.descrip) - 1] = 0; + /* String is purposefully too long */ + strlcpy(reference_header.intent_name,"PHANTOM_DATA to be used for regression testing the nifti reader/writer",sizeof(reference_header.intent_name)); + strlcpy(reference_header.descrip,"This is a very long dialog here to use up more than 80 characters of space to test to see if the code is robust enough to deal appropriately with very long and obnoxious lines.",sizeof(reference_header.descrip)); { int nbyper; @@ -492,7 +490,7 @@ int main (int argc, const char *argv[]) snprintf(buf,sizeof(buf),"nifti_datatype_string %d",constant); \ PrintTest( \ buf, \ - nifti_is_inttype(constant) != rval, \ + nifti_is_inttype(constant) != (rval), \ true, \ &Errors); \ } @@ -620,7 +618,7 @@ int main (int argc, const char *argv[]) nifti_datatype_sizes(constant,&nbyper,&swapsize); \ PrintTest( \ buf, \ - nbyper != Nbyper || swapsize != Swapsize, \ + nbyper != (Nbyper) || swapsize != (Swapsize), \ true, \ &Errors); \ } diff --git a/niftilib/testdata/dup_attr.nia b/niftilib/testdata/dup_attr.nia new file mode 100644 index 00000000..ad9932a3 Binary files /dev/null and b/niftilib/testdata/dup_attr.nia differ diff --git a/niftilib/testdata/n1_overflow.nii b/niftilib/testdata/n1_overflow.nii new file mode 100644 index 00000000..a5d8c10b Binary files /dev/null and b/niftilib/testdata/n1_overflow.nii differ diff --git a/niftilib/testdata/n1_volsize.nii b/niftilib/testdata/n1_volsize.nii new file mode 100644 index 00000000..98160d48 Binary files /dev/null and b/niftilib/testdata/n1_volsize.nii differ diff --git a/string_helper/string_helper.c b/string_helper/string_helper.c new file mode 100644 index 00000000..94d7c42e --- /dev/null +++ b/string_helper/string_helper.c @@ -0,0 +1,58 @@ +#include +#include +#include +#include +#include + +// Apple provides strlcpy and strlcat, so don't redefine them. +#ifndef __APPLE__ + +// libc 2.38 and above provides strlcpy and strlcat, so don't redefine them. +#if defined (__GLIBC__) && ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 38))) + +// --------------------------------------------------------------------------------------------------------------- +size_t strlcpy(char* restrict ioDestination, const char* restrict inSource, size_t inDestinationSize) +{ + assert(inSource); + + size_t sourceLength = strlen(inSource); + + if (inDestinationSize) + { + size_t length = (sourceLength >= inDestinationSize) ? inDestinationSize - 1 : sourceLength; + memcpy(ioDestination, inSource, length); + ioDestination[length] = '\0'; + } + + return sourceLength; +} + + +// --------------------------------------------------------------------------------------------------------------- +size_t strlcat(char* restrict ioDestination, const char* restrict inSource, size_t inDestinationSize) +{ + assert(ioDestination); + assert(inSource); + + size_t destinationLength = strlen(ioDestination); + size_t sourceLength = strlen(inSource); + size_t totalLength = destinationLength + sourceLength; + + assert(destinationLength < inDestinationSize); + + ioDestination += destinationLength; + inDestinationSize -= destinationLength; + if (sourceLength >= inDestinationSize) + { + sourceLength = inDestinationSize - 1; + } + memcpy(ioDestination, inSource, sourceLength); + ioDestination[sourceLength] = '\0'; + + return totalLength; +} + +#endif +#endif + + diff --git a/string_helper/string_helper.h b/string_helper/string_helper.h new file mode 100644 index 00000000..b5ca068f --- /dev/null +++ b/string_helper/string_helper.h @@ -0,0 +1,14 @@ +// Apple provides strlcpy and strlcat, so don't redefine them. +#ifndef __APPLE__ + +// libc 2.38 and above provides strlcpy and strlcat, so don't redefine them. +#if defined (__GLIBC__) && ((__GLIBC__ < 2) || ((__GLIBC__ == 2) && (__GLIBC_MINOR__ < 38))) + +#include + +size_t strlcpy(char* restrict ioDestination, const char* restrict inSource, size_t inDestinationSize); + +size_t strlcat(char* restrict ioDestination, const char* restrict inSource, size_t inDestinationSize); + +#endif +#endif diff --git a/znzlib/CMakeLists.txt b/znzlib/CMakeLists.txt index 10239be6..5be28397 100644 --- a/znzlib/CMakeLists.txt +++ b/znzlib/CMakeLists.txt @@ -27,3 +27,15 @@ if(BUILD_SHARED_LIBS) target_compile_definitions(${NIFTI_ZNZLIB_NAME} INTERFACE ZNZ_USE_SHARED) endif() install_nifti_target(${NIFTI_ZNZLIB_NAME}) + +if(NIFTI_BUILD_TESTING AND ZLIB_FOUND AND NOT WIN32) + # znzprintf is only compiled under COMPILE_NIFTIUNUSED_CODE, and not at + # all on Windows, so the test builds its own copy of znzlib.c with that + # definition and is registered only where the function exists. + add_executable(znzprintf_test znzprintf_test.c znzlib.c) + target_compile_definitions(znzprintf_test PRIVATE COMPILE_NIFTIUNUSED_CODE) + target_include_directories(znzprintf_test PRIVATE ${CMAKE_CURRENT_LIST_DIR} ${ZLIB_INCLUDE_DIR}) + target_link_libraries(znzprintf_test PRIVATE ${NIFTI_ZLIB_LIBRARIES}) + add_test(NAME znzprintf_truncation + COMMAND $ ${CMAKE_CURRENT_BINARY_DIR}/znzprintf_test.gz) +endif() diff --git a/znzlib/znzlib.c b/znzlib/znzlib.c index 52b5665d..90a6a7f8 100644 --- a/znzlib/znzlib.c +++ b/znzlib/znzlib.c @@ -145,9 +145,10 @@ size_t znzread(void* buf, size_t size, size_t nmemb, znzFile file) while( remain > 0 ) { n2read = (remain < ZNZ_MAX_BLOCK_SIZE) ? (unsigned)remain : ZNZ_MAX_BLOCK_SIZE; nread = gzread(file->zfptr, (void *)cbuf, n2read); - if( nread < 0 ) return nread; /* returns -1 on error */ + /* 0, not gzread's -1: this returns size_t, where -1 is SIZE_MAX. */ + if( nread < 0 ) return 0; - remain -= nread; + remain -= (size_t)nread; cbuf += nread; /* require reading n2read bytes, so we don't get stuck */ @@ -178,10 +179,10 @@ size_t znzwrite(const void* buf, size_t size, size_t nmemb, znzFile file) n2write = (remain < ZNZ_MAX_BLOCK_SIZE) ? (unsigned)remain : ZNZ_MAX_BLOCK_SIZE; nwritten = gzwrite(file->zfptr, (const void *)cbuf, n2write); - /* gzread returns 0 on error, but in case that ever changes... */ - if( nwritten < 0 ) return nwritten; + /* gzwrite returns 0 on error, but in case that ever changes... */ + if( nwritten < 0 ) return 0; - remain -= nwritten; + remain -= (size_t)nwritten; cbuf += nwritten; /* require writing n2write bytes, so we don't get stuck */ @@ -297,18 +298,30 @@ int znzprintf(znzFile stream, const char *format, ...) int retval=0; char *tmpstr; va_list va; - if (stream==NULL) { return 0; } + /* the printf family reports failure with a negative value; 0 means an + empty write succeeded, so it cannot be used for the failures below */ + if (stream==NULL) { return -1; } va_start(va, format); #ifdef HAVE_ZLIB if (stream->zfptr!=NULL) { size_t size; /* local to HAVE_ZLIB block */ - size = strlen(format) + 1000000; /* overkill I hope */ + int written; + size = strlen(format) + 1000000; /* still generous, but now a bound */ tmpstr = (char *)calloc(1, size); if( tmpstr == NULL ){ fprintf(stderr,"** ERROR: znzprintf failed to alloc %zu bytes\n", size); - return retval; + va_end(va); + return -1; + } + written = vsnprintf(tmpstr,size,format,va); + if( written < 0 || (size_t)written >= size ){ + /* writing the truncated text would put a partial record in the + file and report it as a complete one, so write nothing */ + fprintf(stderr,"** ERROR: znzprintf output truncated at %zu bytes\n", size-1); + free(tmpstr); + va_end(va); + return -1; } - vsprintf(tmpstr,format,va); retval=gzprintf(stream->zfptr,"%s",tmpstr); free(tmpstr); } else diff --git a/znzlib/znzlib.h b/znzlib/znzlib.h index ff031687..866fc01c 100644 --- a/znzlib/znzlib.h +++ b/znzlib/znzlib.h @@ -145,14 +145,25 @@ ZNZ_API int znzputs(const char *str, znzFile file); #ifdef COMPILE_NIFTIUNUSED_CODE ZNZ_API char * znzgets(char* str, int size, znzFile file); +ZNZ_API int znzflush(znzFile file); + +ZNZ_API int znzeof(znzFile file); + ZNZ_API int znzputc(int c, znzFile file); ZNZ_API int znzgetc(znzFile file); #if !defined(WIN32) +/* the attribute lets the caller's format be checked, which is what makes + the internal vsnprintf on it acceptable to -Wformat-nonliteral */ +#if defined(__GNUC__) || defined(__clang__) +ZNZ_API int znzprintf(znzFile stream, const char *format, ...) + __attribute__((format(printf, 2, 3))); +#else ZNZ_API int znzprintf(znzFile stream, const char *format, ...); #endif #endif +#endif /*=================*/ #ifdef __cplusplus diff --git a/znzlib/znzprintf_test.c b/znzlib/znzprintf_test.c new file mode 100644 index 00000000..24b6c321 --- /dev/null +++ b/znzlib/znzprintf_test.c @@ -0,0 +1,67 @@ +/* Exercises znzprintf(), which is compiled only under + COMPILE_NIFTIUNUSED_CODE and has no caller inside this tree. */ + +#include +#include +#include + +#include "znzlib.h" + +#define BIG_LEN 2000000 + +static const char expected[] = "abc\n"; + +int main(int argc, char *argv[]) +{ + const char *path; + znzFile zf; + char *big; + int normal; + int truncated; + int status = 0; + size_t got; + char readback[64]; + + if( argc < 2 ){ fprintf(stderr,"usage: %s OUTFILE.gz\n", argv[0]); return 1; } + path = argv[1]; + + zf = znzopen(path, "wb", 1); + if( zf == NULL ){ fprintf(stderr,"** cannot open %s\n", path); return 1; } + + normal = znzprintf(zf, "%s", expected); + if( normal != (int)strlen(expected) ){ + fprintf(stderr,"** FAIL: ordinary write returned %d, expected %d\n", + normal, (int)strlen(expected)); + status = 1; + } + + big = (char *)malloc(BIG_LEN + 1); + if( big == NULL ){ fprintf(stderr,"** cannot allocate\n"); znzclose(zf); return 1; } + memset(big, 'x', BIG_LEN); + big[BIG_LEN] = '\0'; + + truncated = znzprintf(zf, "%s", big); + if( truncated >= 0 ){ + fprintf(stderr,"** FAIL: truncating write returned %d, expected a " + "negative value\n", truncated); + status = 1; + } + free(big); + + if( znzclose(zf) != 0 ){ fprintf(stderr,"** cannot close %s\n", path); return 1; } + + zf = znzopen(path, "rb", 1); + if( zf == NULL ){ fprintf(stderr,"** cannot reopen %s\n", path); return 1; } + memset(readback, 0, sizeof(readback)); + got = znzread(readback, 1, sizeof(readback) - 1, zf); + znzclose(zf); + + if( got != strlen(expected) || strcmp(readback, expected) != 0 ){ + fprintf(stderr,"** FAIL: file holds %zu bytes, expected only the %d " + "bytes of the ordinary write\n", got, (int)strlen(expected)); + status = 1; + } + + if( status == 0 ) printf("znzprintf test passed\n"); + return status; +}