|SUCCESS| [1/1] log: add name query functions
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Thu Oct 22 23:11:15 CEST 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/171873
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-22
# DPDK Patch Review
## Patch: log: add name query functions
### ERRORS
None.
---
### WARNINGS
None.
---
### INFO
**Documentation and API Design:**
The patch adds two experimental helper functions to query log level and type names. The API design is reasonable for read-only queries that return DPDK-owned strings. The Doxygen comments correctly document ownership and lifetime, which is good practice for string-returning APIs.
**Implementation Notes:**
1. **`rte_log_get_type_name()` boundary check (line in `lib/log/log.c`):**
```c
if (type >= rte_logs.dynamic_types_len)
return NULL;
```
This correctly validates the type against the array bounds before accessing `rte_logs.dynamic_types[type].name`.
2. **`rte_log_get_level_name()` delegates to `eal_log_level2str()`:**
The function delegates to an existing internal helper. Without seeing the implementation of `eal_log_level2str()`, I cannot verify it handles all edge cases, but the test coverage shows it returns "disabled" for 0 and "unknown" for `UINT32_MAX`, suggesting appropriate fallback behavior.
3. **Test coverage (in `app/test/test_logs.c`):**
```c
TEST_ASSERT(strcmp(rte_log_get_level_name(RTE_LOG_ERR), "error") == 0,
"invalid name for error log level\n");
TEST_ASSERT(strcmp(rte_log_get_level_name(0), "disabled") == 0,
"invalid name for disabled log level\n");
TEST_ASSERT(strcmp(rte_log_get_level_name(UINT32_MAX), "unknown") == 0,
"invalid name for unknown log level\n");
TEST_ASSERT(strcmp(rte_log_get_type_name(logtype1), "logtype1") == 0,
"invalid name for logtype1\n");
TEST_ASSERT_NULL(rte_log_get_type_name(UINT32_MAX),
"invalid log type has a name\n");
```
Good coverage of boundary cases: valid level, zero level, invalid level, valid type, and invalid type.
4. **String ownership documentation:**
The Doxygen comments explicitly state that returned strings are owned by DPDK and must not be modified or freed. This is appropriate for returning pointers to internal static/persistent storage.
5. **Export macros:**
```c
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_log_get_level_name, 26.11)
RTE_EXPORT_EXPERIMENTAL_SYMBOL(rte_log_get_type_name, 26.11)
```
Correctly placed in the `.c` file immediately before the function definitions, with the version indicating first release (26.11).
6. **Release notes:**
The patch appropriately documents the new experimental API in `doc/guides/rel_notes/release_26_11.rst`.
7. **Header guards and tags:**
The `__rte_experimental` tag is correctly placed alone on the line preceding the return type in `lib/log/rte_log.h`.
**Summary:**
The patch is well-structured, includes appropriate test coverage, documents string ownership clearly, and follows DPDK conventions for experimental API introduction. No correctness bugs or style violations identified.
Review-Result: CLEAN
More information about the test-report
mailing list