|WARNING| eal: add some debug for unit test failures
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Oct 16 19:09:34 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/171563
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-16
# DPDK Patch Review
## Errors
None identified.
## Warnings
### 1. Missing `<errno.h>` include for `strerror(errno)` usage
**File:** `app/test/commands.c`
**Line:** `printf("commands_init: calloc(%d) failed %s\n", commands_len, strerror(errno));`
The patch adds a call to `strerror(errno)` without including `<errno.h>`.
While `errno` may be transitively included, explicit inclusion is required for portability.
**Suggested fix:**
```c
#include <errno.h>
```
Add this at the top of `app/test/commands.c` in the appropriate include group (system/libc includes).
---
### 2. Format specifier mismatch for `size_t`
**File:** `app/test/commands.c`
**Line:** `printf("commands_init: calloc(%d) failed %s\n", commands_len, strerror(errno));`
`commands_len` is `size_t` but the format specifier is `%d` (expects `int`).
This can produce incorrect output or warnings on 64-bit systems where `size_t` is larger than `int`.
**Suggested fix:**
```c
printf("commands_init: calloc(%zu) failed %s\n", commands_len, strerror(errno));
```
Use `%zu` for `size_t` values.
---
### 3. Format specifier should use `PRIu64` for `size`
**File:** `lib/eal/common/malloc_heap.c`
**Line:** Added `EAL_LOG(ERR, "Error allocating from heap in legacy mode");`
The log message does not include the requested `size` parameter, which would be valuable debug information.
Since `size` is `size_t` (effectively `uint64_t` on most DPDK platforms), it should use `PRIu64`.
**Suggested fix:**
```c
EAL_LOG(ERR, "Error allocating %"PRIu64" bytes from heap in legacy mode", (uint64_t)size);
```
---
## Info
### Debug logging in hot path
**File:** `lib/eal/common/malloc_heap.c`
Adding `EAL_LOG(ERR, ...)` in `malloc_heap_alloc_on_heap_id()` on every allocation failure
may generate excessive log output if the application routinely attempts allocations that fail
(e.g., trying different memory sizes until one succeeds).
Consider whether this debug should be conditional (e.g., debug build only)
or include rate limiting if it will remain in production code.
For a temporary debug patch this is acceptable, but worth noting for a permanent fix.
---
### Child process status interpretation
**File:** `app/test/process.c`
The added debug output for child process status is useful for debugging but will appear
for every child process spawned by the test suite, including successful cases.
Consider whether the output should be conditional on non-zero exit status:
```c
if (WIFEXITED(status) && WEXITSTATUS(status) != 0)
printf("Child process exited with status %d\n", WEXITSTATUS(status));
```
This is an improvement suggestion for a debug patch, not a correctness issue.
---
### Missing size and flags in debug output
**File:** `lib/eal/common/malloc_heap.c`
The error message would be more useful if it included the allocation parameters (`size`, `flags`, `align`).
See suggested fix in Warning #3.
---
Review-Result: WARNING
More information about the test-report
mailing list