Copilot commented on code in PR #12712:
URL: https://github.com/apache/trafficserver/pull/12712#discussion_r2582463017
##########
src/iocore/cache/unit_tests/main.cc:
##########
@@ -324,14 +324,14 @@ CacheReadTest::read_event(int event, void *e)
if (memcmp(str.data(), this->_cursor, str.size()) == 0) {
this->_reader->consume(str.size());
this->_cursor += str.size();
- this->process_event(event);
} else {
CHECK(false);
this->close();
TEST_DONE();
break;
Review Comment:
The `process_event(event)` call should not be executed when the error path
(lines 328-331) is taken. After `close()` is called on line 329, `vc` and `vio`
are set to `nullptr`, which will cause `REQUIRE` failures in
`handle_cache_event()` when processing `VC_EVENT_READ_READY`. Consider
restructuring to skip `process_event()` after error handling:
```cpp
bool has_error = false;
while (this->_reader->block_read_avail()) {
auto str = this->_reader->block_read_view();
if (memcmp(str.data(), this->_cursor, str.size()) == 0) {
this->_reader->consume(str.size());
this->_cursor += str.size();
} else {
CHECK(false);
this->close();
TEST_DONE();
has_error = true;
break;
}
}
if (!has_error) {
this->process_event(event);
}
```
Alternatively, use `return 0;` instead of `break;` on line 331 to exit the
handler early.
```suggestion
return 0;
```
--
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]
