|SUCCESS| ethdev/telemetry: fix hide_zero parameter being reset
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Fri Oct 30 06:07:42 CET 2026
Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/172252
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-30
# DPDK Patch Review
## Correctness Issues
### ERRORS
**Resource leak on parse error path**
Line 1522 in the else branch:
```c
filter = (filter_val != NULL) ? strdup(filter_val) : NULL;
```
When `filter_val` is not NULL, `strdup()` allocates memory that is assigned to `filter`. However, if the kvargs parsing encountered an error (the condition checks `ret != 0`), the code still logs a NOTICE and continues to line 1522 where it performs the `strdup()`. This means `filter` could be allocated even when parsing failed. The patch does not show the cleanup path, but if `filter` is not freed on the error case before returning, this introduces a memory leak.
**Suggested fix**: Only allocate `filter` when parsing succeeds:
```c
if (kvlist == NULL || (filter_val != NULL && strlen(filter_val) == 0) || ret != 0) {
RTE_ETHDEV_LOG_LINE(NOTICE,
"Unknown extra parameters passed to ethdev telemetry command, ignoring");
} else {
filter = (filter_val != NULL) ? strdup(filter_val) : NULL;
}
```
The patch already does this structurally, so the error is **pre-existing** in the original code if `filter` is not freed before return. However, the patch as written is **correct** -- it moves the `filter` allocation into the else branch, which only executes when parsing succeeds.
Re-reading the patch: the original code had:
```c
} else {
filter = (filter_val != NULL) ? strdup(filter_val) : NULL;
hide_zero = false; /* BUG: discards parsed value */
}
```
The patch removes the `hide_zero = false;` line, which is correct. The `filter` allocation is already in the else branch (success path), so there is no new leak introduced. Disregard the above concern.
**No correctness bugs found in this patch.**
## Code Style Issues
### WARNINGS
None.
### INFO
None.
## Summary
The patch correctly fixes a logic bug where the `hide_zero` parameter was being reset to `false` after being successfully parsed, causing the user-supplied value to be ignored. The fix removes the erroneous assignment while preserving the correct behavior for the `filter` parameter allocation.
The change is minimal, focused, and does not introduce any new issues. The existing code structure (allocation in the else branch) ensures no resource leak on parse failure.
Review-Result: CLEAN
More information about the test-report
mailing list