Skip to content

Clean up code to eliminate Clang Analyzer warnings - #12226

Merged
cmcfarlen merged 2 commits into
apache:masterfrom
hnakamur:cleanup_to_eliminate_clang_analyzer_warnings
May 12, 2025
Merged

Clean up code to eliminate Clang Analyzer warnings#12226
cmcfarlen merged 2 commits into
apache:masterfrom
hnakamur:cleanup_to_eliminate_clang_analyzer_warnings

Conversation

@hnakamur

@hnakamur hnakamur commented May 7, 2025

Copy link
Copy Markdown
Contributor

This commit resolves the following warnings.

analyze-build-18: INFO: Report directory created: /src/trafficserver/build-ci-clang-analyzer/clang-analyzer-output/a0f86ddbbbe5e73c397b46b80d4b2b9cd23c936c/scan-build-2025-05-07-20-58-14-229251-z8_64v4d
analyze-build-18: INFO: /src/trafficserver/src/tscore/ink_queue.cc:177:74: warning: The result of the '/' expression is undefined [core.UndefinedBinaryOperatorResult]
analyze-build-18: INFO:   177 |     f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_pagesize()) / f->type_size;
analyze-build-18: INFO:       |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/src/iocore/net/UnixNetVConnection.cc:676:7: warning: 3rd function call argument is an uninitialized value [core.CallAndMessage]
analyze-build-18: INFO:   676 |       write_signal_error(nh, this, err);
analyze-build-18: INFO:       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/src/iocore/cache/CacheProcessor.cc:984:9: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
analyze-build-18: INFO:   984 |         int smallest     = sorted_vols[i];
analyze-build-18: INFO:       |         ^~~~~~~~~~~~       ~~~~~~~~~~~~~~
analyze-build-18: INFO: /src/trafficserver/src/iocore/cache/CacheProcessor.cc:1008:9: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
analyze-build-18: INFO:  1008 |         int disk_no = sorted_vols[i];
analyze-build-18: INFO:       |         ^~~~~~~~~~~   ~~~~~~~~~~~~~~
analyze-build-18: INFO: 2 warnings generated.
analyze-build-18: INFO: /src/trafficserver/src/proxy/http/HttpTransact.cc:7531:15: warning: Value stored to 'body_type' during its initialization is never read [deadcode.DeadStores]
analyze-build-18: INFO:  7531 |   const char *body_type = "UNKNOWN";
analyze-build-18: INFO:       |               ^~~~~~~~~   ~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/plugins/experimental/memcache/tsmemcache.cc:252:15: warning: Value stored to 'errstr' during its initialization is never read [deadcode.DeadStores]
analyze-build-18: INFO:   252 |   const char *errstr = "Unknown error";
analyze-build-18: INFO:       |               ^~~~~~   ~~~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: WARNING: Run 'scan-view /src/trafficserver/build-ci-clang-analyzer/clang-analyzer-output/a0f86ddbbbe5e73c397b46b80d4b2b9cd23c936c/scan-build-2025-05-07-20-58-14-229251-z8_64v4d' to examine bug reports.

@hnakamur
Clean up code to eliminate Clang Analyzer warnings
938cca4 
This commit resolves the following warnings.
```
analyze-build-18: INFO: Report directory created: /src/trafficserver/build-ci-clang-analyzer/clang-analyzer-output/a0f86ddbbbe5e73c397b46b80d4b2b9cd23c936c/scan-build-2025-05-07-20-58-14-229251-z8_64v4d
analyze-build-18: INFO: /src/trafficserver/src/tscore/ink_queue.cc:177:74: warning: The result of the '/' expression is undefined [core.UndefinedBinaryOperatorResult]
analyze-build-18: INFO:   177 |     f->chunk_size = INK_ALIGN(chunk_size * f->type_size, ats_pagesize()) / f->type_size;
analyze-build-18: INFO:       |                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/src/iocore/net/UnixNetVConnection.cc:676:7: warning: 3rd function call argument is an uninitialized value [core.CallAndMessage]
analyze-build-18: INFO:   676 |       write_signal_error(nh, this, err);
analyze-build-18: INFO:       |       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/src/iocore/cache/CacheProcessor.cc:984:9: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
analyze-build-18: INFO:   984 |         int smallest     = sorted_vols[i];
analyze-build-18: INFO:       |         ^~~~~~~~~~~~       ~~~~~~~~~~~~~~
analyze-build-18: INFO: /src/trafficserver/src/iocore/cache/CacheProcessor.cc:1008:9: warning: Assigned value is garbage or undefined [core.uninitialized.Assign]
analyze-build-18: INFO:  1008 |         int disk_no = sorted_vols[i];
analyze-build-18: INFO:       |         ^~~~~~~~~~~   ~~~~~~~~~~~~~~
analyze-build-18: INFO: 2 warnings generated.
analyze-build-18: INFO: /src/trafficserver/src/proxy/http/HttpTransact.cc:7531:15: warning: Value stored to 'body_type' during its initialization is never read [deadcode.DeadStores]
analyze-build-18: INFO:  7531 |   const char *body_type = "UNKNOWN";
analyze-build-18: INFO:       |               ^~~~~~~~~   ~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: INFO: /src/trafficserver/plugins/experimental/memcache/tsmemcache.cc:252:15: warning: Value stored to 'errstr' during its initialization is never read [deadcode.DeadStores]
analyze-build-18: INFO:   252 |   const char *errstr = "Unknown error";
analyze-build-18: INFO:       |               ^~~~~~   ~~~~~~~~~~~~~~~
analyze-build-18: INFO: 1 warning generated.
analyze-build-18: WARNING: Run 'scan-view /src/trafficserver/build-ci-clang-analyzer/clang-analyzer-output/a0f86ddbbbe5e73c397b46b80d4b2b9cd23c936c/scan-build-2025-05-07-20-58-14-229251-z8_64v4d' to examine bug reports.
```
@hnakamur

hnakamur commented May 7, 2025

Copy link
Copy Markdown
Contributor Author

The following two tests failed.

However all tests passed on my local PC.

@JosiahWI JosiahWI added this to the 10.2.0 milestone May 7, 2025
bneradt
bneradt requested changes May 7, 2025
View reviewed changes

@bneradt bneradt left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the fixes.

Comment thread src/proxy/http/HttpTransact.cc Outdated
Comment thread src/iocore/net/UnixNetVConnection.cc Outdated
Comment thread plugins/experimental/memcache/tsmemcache.cc Outdated
@hnakamur
hnakamur requested a review from bneradt May 9, 2025 00:29
@hnakamur

hnakamur commented May 9, 2025

Copy link
Copy Markdown
Contributor Author

Sorry, "hnakamur requested a review from bneradt" was my mistake.

@cmcfarlen
cmcfarlen merged commit 0cf0f3e into apache:master May 12, 2025
@hnakamur
hnakamur deleted the cleanup_to_eliminate_clang_analyzer_warnings branch May 15, 2025 08:10
This was referenced Sep 15, 2026
Merged
Merged
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Sep 15, 2026
@hnakamur @cmcfarlen
Clean up Clang analyzer warnings (apache#12226)
f93d952 
Uninitialized values and redundant initializations cause analyzer
warnings in the 10.1.x branch.

This patch initializes the affected locals and uses a value-initialized
container for volume sorting. It backports apache#12226.

(cherry picked from commit 0cf0f3e)
cmcfarlen pushed a commit to cmcfarlen/trafficserver that referenced this pull request Sep 15, 2026
bneradt @cmcfarlen
Restore Clang analyzer checks on 10.1.x
7fb5ab1 
Clang 21 still flags the unguarded insertion-sort path used for SRV
records, even after the pointer array is initialized. This keeps the
10.1.x analyzer job failing after the earlier analyzer fixes.

This branch backports apache#13593 and
apache#12226, and uses stable sorting with the existing
SRV comparator. The sort preserves priority/key ordering while avoiding
the analyzer's unguarded-sort false positive.
@github-project-automation github-project-automation Bot moved this to For v10.1.1 in ATS v10.1.x Sep 15, 2026
ezelkow1 pushed a commit that referenced this pull request Sep 15, 2026
@bneradt @hnakamur
@duke8253 @bryancall bneradt
9.2.x: latest clang-analyzer fixes (#13692)
0a7cfc9 
* Backport Clang 21 analyzer fixes to 9.2.x

Newer Clang releases expose ownership and error-handling issues in
9.2.x as well as file I/O performed while holding the body factory lock.

This patch adapts the applicable fixes from #13593 to the older source
layout and APIs. CMake and changes to code absent from 9.2.x are omitted.

(cherry picked from commit c31517c)

* Backport earlier Clang analyzer fixes to 9.2.x

The older branch still contains uninitialized error values and cache
volume indices flagged by the analyzer.

This patch adapts #12226 to initialize those values, verify the
freelist element size before division, and remove dead initializers.

(cherry picked from commit 0cf0f3e)

* Backport session reuse key-file checks

Missing or unreadable Redis authentication key files can leave invalid
file descriptors and read lengths in the session reuse plugin.

This patch backports the key-file checks from #10273 and closes the
file only when it was opened successfully. Configuration-file handling
is addressed separately for the older branch.

(cherry picked from commit 26affda)

* Backport header rewrite rule ownership fix

Incomplete header rewrite rules can leak when configuration parsing
ends without handing the rule to the configuration.

This patch adapts #11386 to keep temporary rules under unique ownership
until the configuration accepts them.

(cherry picked from commit 0887836)

* 9.2.x: latest clang-analyzer fixes

The Ubuntu 26.04 clang-analyzer job exposes unchecked I/O, a rule leak,
and configuration reads under a mutex in the older 9.2.x code.

This branch backports the applicable fixes from #13593, #12226, #10273,
and #11386. The remaining changes handle file and socket failures,
keep session reuse configuration I/O outside the reader lock, and make
the nonblocking eventfd operation explicit to the analyzer.

* Address review findings and isolate TLS autest

A full-size Redis key leaves no room for its terminator, and failed
configuration reloads can repeat file I/O on every lookup. Modern
Fedora policy also prevents the TLS autest from exercising TLS 1.0.

This patch tightens the key bound, restores timestamp-gated reloads,
clarifies configuration publication, and guards device-dependent
checks. The TLS test uses isolated OpenSSL settings and RSA key
exchange so all four protocol checks remain active.

---------

Co-authored-by: Hiroaki Nakamura <hnakamur@gmail.com>
Co-authored-by: Fei Deng <feid@yahooinc.com>
Co-authored-by: Bryan Call <bcall@apache.org>
Co-authored-by: bneradt <bneradt@yahooinc.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cmcfarlen cmcfarlen cmcfarlen approved these changes
@bneradt bneradt bneradt approved these changes

Assignees

@hnakamur hnakamur

Labels

clang-analyzer Cleanup

Projects

ATS v10.1.x
Status: For v10.1.1

Milestone

10.2.0

Development

Successfully merging this pull request may close these issues.

4 participants

@hnakamur @cmcfarlen @bneradt @JosiahWI