Repository navigation
small GCC fixes - #147
Merged
Merged
small GCC fixes#147
Conversation
added 6 commits
October 17, 2026 19:02
The resolved NDIlib_destroy function pointer was never called, leaving the loaded NDI runtime torn down only by the OS at process exit rather than explicitly. NDIlib_destroy is a one-time pair with NDIlib_initialize, so it is registered via std::atexit the first time initialization succeeds, not at any sender close (a sender may reopen later in the same run). Core: platform_desktop.cpp calls NDIlib_destroy via std::atexit once NDI initializes successfully.
The clear() test discarded allocate()'s return value before indexing data(), so a failed allocation would have written through a null pointer with no test failure pointing at the real cause. The accessor test right above this one already checks it; this one now matches. Tests: unit_Buffer.cpp requires the allocation to succeed before using the buffer it returns.
GCC 16.2.1 reopens -Wnull-dereference on JsonDoc::rootNode(), past this file's existing version-15 bound. Two of the flagged call sites turned out to be real: StateDocument.cpp's applyStateDocument/writeFlatValues and ControlModule.h's readHeader all dereference rootNode() only after a json::parse() success, which guarantees a non-null root by construction (verified against parse()'s own implementation), but neither call site said so explicitly. Both now check the pointer directly rather than relying on an invariant visible only in a different function. A third class of site (MoonModule::enabled()'s plain this->member_ read, reached through several tests' std::function plumbing) has no such fix: GCC loses track of `this` across the inlined lambda chain, and testing `this == nullptr` in a member function isn't meaningful C++. The remaining exception is scoped to that shape only, not the two fixed ones. Core: StateDocument.cpp and ControlModule.h null-check their rootNode() result explicitly instead of relying on an invariant from a sibling function. CMakeLists.txt's -Wno-error=null-dereference for GCC is now justified against one real, unfixable false-positive class rather than whatever GCC happens to flag.
MoonModule::setName's std::memcpy(name_, n, len) is bound-checked (len < sizeof(name_), a real fixed array) before the call, but GCC 16.2.1, inlining it through several unit tests' DOCTEST_ANON_FUNCs, deduces a 0-byte region for name_ at some of those call sites and reports offset 0 as out of [0, 0] bounds — the same aggressive-inlining-loses-the-real-size class the file's existing GCC accommodations already name for other sites. Docs/CI: CMakeLists.txt adds -Wno-error=array-bounds= for GCC.
…d loop unit_Hub75Driver_pins.cpp reads out[i] only for i < n, where n is fixedPins(out, 16)'s own return value (checked == 14 in the same test), but GCC 16.2.1 still flags out[14]/out[15] as possibly uninitialized — past the loop's own dynamic bound, not a real read of memory the loop never reaches. GCC isn't correlating n's value with the loop condition after inlining, the same class as the file's other new GCC 16.2.1 accommodations. Docs/CI: CMakeLists.txt adds -Wno-error=maybe-uninitialized for GCC.
Vendored doctest.h (v2.4.11) always includes <ciso646> on Clang, unconditionally, to detect libc++ via _LIBCPP_VERSION (doctest's own long-standing quirk: github.com/doctest/doctest/issues/126, #356). A Clang new enough to deprecate that pre-C++20 header turns its #warning into a build failure under -Werror. Third-party header code we don't own, so this stays unconditional rather than version-gated, matching the existing MSVC C5285 suppression for the same vendored file. Docs/CI: CMakeLists.txt adds a Clang-only -Wno-error=#warnings.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configuration
You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
A quit closes the sender before the exit hook runs, so destroying the runtime there is the clean pairing with NDIlib_initialize. A reboot exits with the render thread still sending, where destroying the runtime under a live sender races it; there the process ending is the teardown.
Collaborator
|
Thanks @TouchMyLight, good to have GCC 16 covered, and the two We added one commit to your branch: the NDI teardown now runs only when no sender is open, since a reboot exits while the render thread may still be sending. After merging we'll gate the GCC suppressions on the version that needs them and scope the test-only ones to the test targets. Looking forward to the FSEQ PRs! |
Collaborator
|
you can also contact me on discord , our server is https://discord.gg/WuQ5zzex3F |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I've upgraded to GCC 16.2.1 and could not build the project anymore. Here some changes to fix the small issues