phongn commented on PR #13257:
URL: https://github.com/apache/trafficserver/pull/13257#issuecomment-5687073013
Follow-up in aaf29f1357 for the two CI failures on 421b518aac.
## Clang-Analyzer — fixed
`deadcode.DeadStores` on the `codec_error` sentinel I added for the
decode-detail reporting:
```
RamCacheCLFUS.cc:308:15: warning: Value stored to 'codec_error' during its
initialization is never read
```
Correct, and the sentinel was load-bearing for nothing: all seven paths to
`Lfailed` assign a real detail string first, so `"no detail"` was never read. I
didn't just delete the initializer — that trades a harmless dead store for a
real uninitialized read the first time someone adds a `goto Lfailed` without
assigning.
Instead I removed the reason the sentinel existed. The `const char *` and
the function-scope `char[128]` were only there because the detail had to
survive a `goto`, so each site now reports before jumping, with its buffer
scoped to the branch:
```cpp
if (l != rc) {
char detail[128];
snprintf(detail, sizeof(detail), "LZ4_decompress_safe returned %d,
expected %d", rc, l);
note_decompress_failure(stripe, key, e, detail);
goto Lfailed;
}
```
A new file-local `note_decompress_failure()` holds the throttled `Warning`
and both counters, and `Lfailed` is back to what it looked like before this PR
touched it — `ats_free(b); this->_destroy(e);` plus the debug trace. Behaviour
is unchanged: `static Throttler` inside the helper is still one process-wide
throttler, and the helper is called while the entry is intact, so the warning
reads the same `e->` fields it read from inside the old `Lfailed` block. Net is
7 fewer lines and two fewer function-scope locals.
This supersedes the `codec_error` snippet in my previous comment — same
information reaches the log, different plumbing.
Verified locally with `clang-tidy --checks='-*,clang-analyzer-*'` against a
`compile_commands.json` (reproduced the warning first, confirmed it gone
after), plus the `.clang-tidy-ci` check set clean on the file, and the cache
tests green in all three configurations including ASan.
## AuTest 2of4 — not this PR
The only failing test in that shard is `rate_limit_sni_queue`, on the
precondition assertion:
```
file .../ts/log/traffic.out : a connection was queued - Failed
Reason: ... did not contains expression: "Queueing the VC"
```
That is #13679, opened today: the queue precondition races a TLS handshake
on a 0.3s timer. The shard was also heavily loaded on this run (worker 1 took
~1456s), which is the condition that issue describes. Nothing in this PR
reaches the SNI rate limiter — the diff is RAM cache compression, a records
validity pattern, and two counters — and every other shard passed, as did all
nine platform builds.
I'd rather not rebase just to reroll the dice on a known flake, so unless
you want it rerun for a clean board, the new push should sort it on its own.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
