|WARNING| dev: hide internal type for device comparison
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Mon Jul 6 18:13:33 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166583
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-06
# DPDK Patch Review
## Patch Summary
This patch moves `rte_dev_cmp_t` from `rte_dev.h` (public API) to `bus_driver.h` (internal API), claiming it's only used in bus operations and not referenced by any public API.
---
## FINDINGS
### Errors
**1. Missing release notes for ABI change**
Removing `rte_dev_cmp_t` from `rte_dev.h` is an ABI break for any application that used this type. The patch must document this in the current release notes under an API/ABI changes section.
**Fix:** Add entry to `doc/guides/rel_notes/release_25_07.rst` (or current release):
```rst
API Changes
-----------
* **Moved rte_dev_cmp_t to internal header.**
The ``rte_dev_cmp_t`` type has been moved from ``rte_dev.h`` to
``bus_driver.h`` as it is only used by bus driver implementations
and not by applications.
```
---
### Warnings
**1. Forward declaration style inconsistency**
The patch adds forward declarations to `bus_driver.h`:
```c
struct rte_dev_iterator;
struct rte_devargs;
struct rte_device;
struct rte_driver;
```
However, only `struct rte_dev_iterator` and `struct rte_driver` are new additions. The existing code already had `struct rte_devargs` and `struct rte_device` visible (they were brought in via the now-removed `#include <rte_dev.h>`).
While the forward declarations are correct, verify that all users of `bus_driver.h` that need the full definitions of these types have appropriate includes. The test file correctly adds `#include <rte_dev.h>`, but check if any other code paths exist.
**Fix:** Audit all `#include <bus_driver.h>` users to ensure they include headers for any types they use from the forward-declared structures.
---
### Info
**1. Documentation duplication**
The Doxygen comment for `rte_dev_cmp_t` is copied verbatim between the two locations. This is appropriate for the move, but ensure the comment remains synchronized if future edits occur. Consider adding a note that this is an internal type if that's the intended scope.
**2. Include order verification**
The test file `app/test/test_vdev.c` now includes `rte_dev.h` before `bus_driver.h`. This is correct per the include ordering guidelines (DPDK EAL includes before misc library includes). Verify this pattern is followed wherever `bus_driver.h` is included and needs `rte_dev_cmp_t`.
---
## Summary
**Must fix (Error):**
- Add release notes entry documenting the API change
**Should verify (Warning):**
- Confirm all `bus_driver.h` users that need full struct definitions have appropriate includes
**Code quality:**
- The approach is sound: moving an internal type from public to internal headers reduces API surface
- Forward declarations are used correctly
- Test file includes updated appropriately
The patch is a reasonable cleanup but requires documentation of the ABI/API change before merging.
More information about the test-report
mailing list