style(mcp): compare nullable strings through one helper - #2396
Conversation
cppcheck 2.20 (the version the lint job pins) reports
knownConditionTrueFalse on two comparators in src/mcp/mcp.c whose
tie-breakers compare one strcmp result after another, each call with
inline "x ? x : \"\"" fallbacks. Neither function differs between these trees; which of
them is flagged depends on the rest of the lint source list. With
scripts/lint.sh --ci and cppcheck 2.20.0 built from source:
- main itself fails on search_result_cmp ("file_order != 0 is always
false");
- adding one more language grammar and its sources moves the finding
to project_record_compare ("by_root != 0 is always false");
- carrying each comparator's order in one variable instead still fails,
now as redundantAssignment on project_record_compare.
Both comparators now call nullable_strcmp(), which orders NULL like ""
exactly as the inline fallbacks did. lint-cppcheck passes again on
main and with the additional sources. Comparison order and results are
unchanged for every input.
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Signed-off-by: Sarlette <282189207+SarletteD@users.noreply.github.com>
|
The first PR of the Structured Text stack that builds on this one is #2397 (refs #1810). Written with Claude Code (Anthropic) on behalf of @SarletteD. |
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Merged. Thank you, @SarletteD! This is a model of how to handle an analyzer false positive: refactor rather than suppress, with a reproduction table showing exactly which tree trips which finding. Each of the five replaced comparisons is the same expression as before ( |
What does this PR do?
Two comparators in
src/mcp/mcp.cbreak ties with onestrcmpafter another, each call spelling outx ? x : ""fallbacks inline:project_record_compare(name, root path, database file) andsearch_result_cmp(score, qualified name, file, then lines). cppcheck 2.20, the version_lint.ymlpins,reports
knownConditionTrueFalseon them ("by_root != 0is always false", "file_order != 0is alwaysfalse"), although neither function differs between these trees: whether it fires depends on the rest of the lint source
list. Both comparators now call
nullable_strcmp(), which orders NULL like""exactly as the inlinefallbacks did. Comparison order and results are unchanged for every input.
With cppcheck 2.20.0 built from source,
mainitself already failslint-cppcheckonsearch_result_cmp(table below); the upstream CI job may or may not hit the same path. It is also a prerequisite for the Structured Text stack (four stacked PRs; the first one is linked in a comment once it is open), which is stacked on it: with the ST
grammar and sources in the tree,
lint-cppcheckturns red onsrc/mcp/mcp.c, a file the stack does nottouch. It is sent on its own because it is a fix, not part of the feature (CONTRIBUTING.md, "Don't mix
features with fixes").
Reproduction (
scripts/lint.sh --ci, clang-format 20.1.8, cppcheck 2.20.0)main@ 64c23famcp.c:12614 Condition 'file_order!=0' is always false [knownConditionTrueFalse]insearch_result_cmplang_specs.c, extraction)mcp.c:2734 Condition 'by_root!=0' is always false [knownConditionTrueFalse]inproject_record_comparemain+ onlyproject_record_comparerewritten (one variable carrying the order)mcp.c:12613 Condition 'file_order!=0' is always falseinsearch_result_cmpmcp.c:2731 Variable 'order' is assigned an expression that holds the same value [redundantAssignment]The findings are false positives: each flagged
strcmpcompares different fields. Routing thecomparisons through one helper removes the inline ternaries the analyzer reasons about.
Scope
src/mcp/mcp.conly,nullable_strcmpplus its two callers (+10 / -6 lines).Commit
style(mcp): compare nullable strings through one helperChecks
./build/c/test-runner mcp: rc 0 (126 PASS lines)scripts/lint.sh --ci(clang-format 20.1.8, cppcheck 2.20.0): rc 0; also rc 0 on every PR of the ST stack on top of itmake -f Makefile.cbm lint-tidy-diff(clang-tidy 21.1.6, diff against main): rc 0Checklist
git commit -s)scripts/lint.sh --ci)Written with Claude Code (Anthropic) on behalf of @SarletteD, who reviewed and signs off every commit.