|SUCCESS| eal: enforce internal API check with Clang
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Oct 15 01:10:09 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/171380
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-14
# DPDK Patch Review
## Summary
This patch reorders the preprocessor logic for `__rte_internal` macro definition to prefer Clang's `diagnose_if` attribute over the `error` attribute, improving detection of internal API misuse.
## ERRORS
None identified.
## WARNINGS
None identified.
## INFO
**Correctness**: The change is sound. The patch swaps two mutually exclusive `#if` branches, maintaining the same fallback chain (`diagnose_if` - `error` - no-op) but changing the preference order. Both branches are guarded by `!defined ALLOW_INTERNAL_API`, and the conditions (`__has_attribute(diagnose_if)` vs `__has_attribute(error)`) are mutually exclusive for the relevant toolchains, so no behavioral change occurs when `ALLOW_INTERNAL_API` is defined or when neither attribute is available.
**Logic preservation**: The transformation is strictly an ordering change. The `diagnose_if` branch (lines 35-45 in the new version) is identical to the old lines 45-55, and the `error` branch (lines 47-55 in the new version) matches the old lines 34-44. The `#else` fallback and MSVC guards remain unchanged.
**Testing note** (pre-existing, not introduced by this patch): The `__rte_internal` macro is tested by attempting to call an internal function without `ALLOW_INTERNAL_API`, but this is a compile-time check. Verification that the change achieves the stated goal -- that Clang now catches more internal API violations -- would require building representative code with both old and new logic and confirming that the new version rejects calls the old version missed. This verification is outside the scope of code review but is something the patch submitter should confirm in testing.
---
Review-Result: CLEAN
More information about the test-report
mailing list