|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