Modernize DpCatalog: in-class initializers and =default destructor#5068
Open
carlosvales wants to merge 2 commits intonasa:develfrom
Open
Modernize DpCatalog: in-class initializers and =default destructor#5068carlosvales wants to merge 2 commits intonasa:develfrom
carlosvales wants to merge 2 commits intonasa:develfrom
Conversation
Resolves several static analysis findings from nasa#4521 in Svc/DpCatalog: - 22x cpp:S3230 ("Do not use the constructor's initializer list for data member ...; use the in-class initializer instead"). All scalar and pointer members previously initialized in the constructor's member-initializer list are now default-initialized in the class body of DpCatalog.hpp. - 1x cpp:S3490 ("Use =default instead of the default implementation of this special member function"). The empty destructor is now declared = default in the header. Behavior is unchanged: members are still initialized in declaration order, which matched the previous initializer-list order, so the construction sequence is identical. Object members (m_directories[], m_fileList[], m_stateFile, m_currXmitFileName) are unchanged because they are default-constructed and were not flagged by SonarQube. Aligned with the in-class initializer style already used elsewhere in the repo (e.g. Svc/DpWriter, Svc/SeqDispatcher, Svc/Ccsds/AosFramer).
Contributor
There was a problem hiding this comment.
Pull request overview
This PR modernizes Svc::DpCatalog construction/destruction by moving scalar/pointer member initialization from the constructor member-initializer list into in-class default member initializers, and by defaulting the (previously empty) destructor. This aligns DpCatalog with existing in-repo patterns and resolves the referenced SonarQube findings without changing runtime behavior.
Changes:
- Convert
~DpCatalog()from an empty out-of-line definition to~DpCatalog() = default;in the header. - Replace constructor member-initializer list for scalar/pointer members with in-class default initializers.
- Simplify the constructor definition to only initialize the base component.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
Svc/DpCatalog/DpCatalog.hpp |
Default the destructor and add in-class default initializers for scalar/pointer members. |
Svc/DpCatalog/DpCatalog.cpp |
Remove the large member-initializer list and the empty destructor definition; keep a minimal constructor delegating to the base. |
Addresses CodeQL "More than one statement per line" alert introduced by
the previous commit, which collapsed the constructor signature and the
empty body onto the same line. Adds an explanatory comment inside the
body to keep it on its own line; clang-format would otherwise re-collapse
the empty `{}` under the repo's Chromium-based style.
Behavior unchanged.
Collaborator
|
What version of C++ does this feature rely on? |
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
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.
Change Description
Migrates the initialization of all scalar/pointer data members of
Svc::DpCatalogfrom the constructor's member-initializer list to in-class default member initializers, and converts the empty destructor to= default.The constructor body is now:
Object members (
m_directories[],m_fileList[],m_stateFile,m_currXmitFileName) are unchanged because they are default-constructed and were not flagged.Rationale
Resolves a cluster of SonarQube findings listed in #4521:
cpp:S3230— "Do not use the constructor's initializer list for data member ...; use the in-class initializer instead." (Medium)cpp:S3490— "Use=defaultinstead of the default implementation of this special member function." (High)The change is intentionally narrow and self-contained:
Svc/DpCatalogconstructor/destructor and member declarations.m_directoriesloop), DpCatalog - validate fdp files #5042 (validate fdp files), or Add destCapacity DeserializeTo for more robustness #5043 (destCapacitydeserialize), all of which target other regions of the same file.This pattern (in-class member initializers and
= default) is already used elsewhere in the repo, e.g.Svc/DpWriter/DpWriter.hpp,Svc/SeqDispatcher/SeqDispatcher.hpp,Svc/Ccsds/AosFramer/AosFramer.hpp,Svc/Ccsds/ApidManager/ApidManager.hpp.Testing/Review Recommendations
Behavior is unchanged — this is a refactor of where members are initialized, not what they are initialized to:
nullptrvalue. The new defaults match exactly:bool → false, integer types →0, pointer types →nullptr.~DpCatalog() {}is semantically equivalent to~DpCatalog() = default;.Local validation:
clang-format --dry-run --Werror(clang-format 22) against the repo's.clang-format(Chromium, IndentWidth 4, ColumnLimit 120): passes with exit 0.cppcheck --enable=warning,style,performance --std=c++17againstSvc/DpCatalog/DpCatalog.{cpp,hpp}reports the same two pre-existing style findings (redundantInitializationonresponseandstat) before and after this change, just shifted by 25 lines because of the line delta. No new findings introduced.Future Work
The remaining static analysis findings in #4521 — cognitive complexity in
fillBinaryTree, nested control flow,reinterpret_castreplacements,std::stringmigrations, const-correctness on parameters and helpers, unchecked parameter dereferences, etc. — remain untouched and can be tackled in follow-up PRs.AI Usage (see policy)
Claude Code (Anthropic, model
claude-opus-4-7) was used as a pair-programming assistant for the following:Svc/DpCatalog/DpCatalog.cppandSvc/DpCatalog/DpCatalog.hpp..clang-formatconfig, and existing repo patterns (Svc/DpWriter,Svc/SeqDispatcher,Svc/Ccsds/AosFramer,Svc/Ccsds/ApidManager) to confirm style alignment.clang-formatandcppcheckwere used to validate the result.