Skip to content

16 compilation error using clang on mac - #17

Open
mmodat wants to merge 2 commits into
masterfrom
16-compilation-error-using-clang-on-mac
Open

16 compilation error using clang on mac#17
mmodat wants to merge 2 commits into
masterfrom
16-compilation-error-using-clang-on-mac

Conversation

@mmodat

@mmodat mmodat commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Fix Clang/C++17 compilation on macOS, plus static-analysis fixes and warning cleanup.
I ended up running cppcheck :)

@mjorgecardoso some these changes change output, please have a glance

Compilation fixes

  • Remove the register storage-class specifier throughout seg-lib/seg-apps (rejected by recent Clang on macOS).
  • Replace deprecated sprintf with bounded snprintf (22 call sites).
  • Modernize CMake bootstrap: cmake_minimum_required(VERSION 3.10) before PROJECT(), drop conflicting 2.8/3.0.2 declarations, set CMP0135 when available.

Bug fixes found by static analysis (cppcheck 2.13, g++ -Wall -Wextra)

  • seg_PatchMatch: k-NN result sorting compared PatchMatchResult* pointers (i.e. heap addresses) instead of dereferencing to the overloaded ANN-distance comparisons, so KNN results were effectively unsorted (_seg_PatchMatch.cpp::sortResults). 😱
  • seg_PatchMatch: the same sort used vector::reserve() as if it were resize(), writing into size-0 vectors (undefined behavior).
  • PatchMatchResult: copy constructor/assignment corrupted the copied values by routing them through buffer(), which pulled from never-copied fields; they now copy all members directly.
  • seg_LoAd: an always-false condition (x > p && x < p) disabled second-best-class tracking in the partial-volume fractional-content step; corrected to compare against the best-class probability.
  • seg_EM: the "Initial Loglik" verbose message was unreachable due to a redundant iter>0 guard.

Hardening

  • Initialize all remaining uninitialized members in seg_PatchMatch, seg_fill_lesions, seg_fill_lesions_other, seg_EM constructors and copy operations; value-initialize the Priors_temp pointer array in seg_EM.
  • Delete copy construction/assignment on seg_EM and seg_LabFusion (they own raw allocations; shallow copies would double-free).
  • Pass std::string/std::vector<std::string> parameters by const reference.
  • Remove unused parameters/variables; misc cleanups (self-assignment no-op, postfix→prefix iterator increment, vestigial always-true output-extension checks).

@mmodat
mmodat requested a review from mjorgecardoso August 12, 2026 12:45
@mmodat mmodat linked an issue Aug 12, 2026 that may be closed by this pull request
@mmodat

mmodat commented Aug 12, 2026

Copy link
Copy Markdown
Contributor Author

Forgot to mention, the downloading of Eigen is also broken as it pulled from cmicdev at the time. I left it has it is and pointed cmake to the version in niftyreg/third-party.

@mmodat

mmodat commented Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

@mjorgecardoso Up!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Compilation error using clang on mac

1 participant