github-actions[bot] commented on PR #61170:
URL: https://github.com/apache/doris/pull/61170#issuecomment-4029370770
## Code Review Summary
**Verdict: No issues found.** The change is correct, minimal, and
well-targeted.
### Critical Checkpoint Conclusions
1. **Goal / Correctness**: The PR fixes ODR (One Definition Rule) violations
for `MockFileReader` across 4 test files by wrapping each definition in an
anonymous namespace, giving them internal linkage. This is the standard and
correct C++ solution. The ASAN heap-buffer-overflow and
new-delete-type-mismatch errors described in the PR should be resolved.
2. **Minimal and focused**: Yes. Core change is adding `namespace { ... }`
around each `MockFileReader` class (4 files, +8/-10 lines). The removed code
(`set_read_status`/`_read_status` in packed_file_reader_test.cpp,
`last_reader()`/`_last_reader` in packed_file_system_test.cpp) is confirmed
dead code with no remaining usages.
3. **Completeness**: All 4 `MockFileReader` definitions in the test codebase
are covered by this fix.
4. **Concurrency**: Not applicable (test-only mock classes).
5. **Lifecycle / static init**: Not applicable. Anonymous namespaces
correctly provide internal linkage.
6. **Parallel code paths**: Verified all instances are fixed.
7. **Test coverage**: The fix is for test infrastructure itself. Existing
tests continue to function correctly.
### Minor Observation (not blocking)
`MockFileWriterForMerge` is defined in both `packed_file_writer_test.cpp`
and `packed_file_system_test.cpp` with identical member layouts but different
method implementations (e.g., `close()` returns `_close_status` vs
`Status::OK()`, `appendv()` checks `_append_status` vs not). This is a similar
ODR violation pattern. While member layout matching prevents the same ASAN
symptoms, the differing vtable implementations are technically still an ODR
violation. Consider wrapping these in anonymous namespaces as a follow-up.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
