bryancall opened a new pull request, #13687:
URL: https://github.com/apache/trafficserver/pull/13687

   Coverity reports "argument cannot be negative" and "out-of-bounds access" on 
values these tests already guard with `REQUIRE`. This PR narrows each one 
explicitly. Part of #13682.
   
   ### The one thing to decide
   
   **Catch2's `REQUIRE` does not constrain values for Coverity.** 
`REQUIRE(expr)` expands to:
   
   ```cpp
   Catch::AssertionHandler catchAssertionHandler(...);
   catchAssertionHandler.handleExpr( Catch::Decomposer() <= expr );
   catchAssertionHandler.complete();
   ```
   
   `complete()` is **not** `[[noreturn]]` — it returns normally when the 
assertion passes — and the expression itself disappears into an opaque 
`ITransientExpression`. So the analyzer has no path from `REQUIRE(n > 0)` back 
to a constraint on `n`. (`Catch::throw_test_failure_exception()` *is* 
`[[noreturn]]`, but it sits behind `complete()`, so a `ci/coverity-model.cpp` 
entry cannot express this without lying about the passing path.)
   
   Accept that, and the remaining 14 commits are the same shape.
   
   ### The shape of every fix
   
   Each site compares against **the test table's own expected length** rather 
than the API's return value, with the existing assertion left intact on the 
line above. The cast is never applied to the API return value, so a negative 
error return is still caught by the `REQUIRE` — nothing is hidden, and no 
assertion was changed, weakened or removed.
   
   | File | CIDs |
   |---|---|
   | `test_LogEntryJson.cc` | 1660665, 1660666, 1660667, 1660668, 1660669, 
1660670, 1660671 |
   | `test_LogAccess.cc` | 1685407 |
   | `test_HpackIndexingTable.cc` | 1644291, 1644272 |
   | `test_Huffmancode.cc` | 1644220, 1644238, 1644269, 1660642 |
   | `test_XPACK.cc` | 1644253, 1644266, 1644292, 1644236 |
   
   Four commits close more than one CID, because Coverity reports one CID per 
pointer argument to a `memcmp` and a single bound fixes both.
   
   ### Also useful to record
   
   Coverity names `TEST_CASE`/`SECTION` bodies `dummyFunctionN`, numbered 
0-based and stepping by 2 per `TEST_CASE`; `SECTION`s are not separately 
numbered. That mapping was confirmed three independent ways (the only body 
containing `rand()`, the only bodies feeding an API return into `memcmp`, and 
matching defect-type counts) before any edit was made.
   
   ### Verification
   
   `test_records`, `test_tsutil`, `test_proxy_hdrs`, `test_proxy_hdrs_xpack`, 
`test_cache`, `test_hostdb`, `test_tscore`, `test_tsconfig` — 321 tests, all 
passing. Every touched file compiles independently on this branch.
   


-- 
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]

Reply via email to