|WARNING| graph: fix xstats of cloned nodes
dpdklab at iol.unh.edu
dpdklab at iol.unh.edu
Wed Jul 8 14:01:56 CEST 2026
Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/166628
_AI Code Review_
=== Patch Review: series.patch (via Claude) ===
Review date: 2026-07-08
# Review of DPDK Patch: graph: fix xstats of cloned nodes
## Summary
This is a one-line fix that copies the `nb_xstats` field when cloning a node with xstats enabled. The patch is correct and addresses a real bug where cloned nodes would report zero xstats despite having copied the xstat descriptors.
---
## Errors
None.
---
## Warnings
### 1. Missing Bounds Check on Array Access
**File:** lib/graph/node.c
**Location:** Line 211 (new line 212 in patched version)
The loop iterates `node->xstats->nb_xstats` times and accesses `node->xstats->xstat_desc[i]`, but there is no verification that `node->xstats->xstat_desc` has enough entries allocated. If the allocation at line 205-208 succeeded but allocated fewer descriptors than `node->xstats->nb_xstats` indicates (due to a bug elsewhere or memory corruption), this could cause an out-of-bounds read.
**Context of existing code:**
```c
reg->xstats->xstat_desc = calloc(node->xstats->nb_xstats,
sizeof(*node->xstats->xstat_desc));
if (reg->xstats->xstat_desc == NULL) {
graph_err("failed to alloc xstat_desc for node %s", name);
goto free;
}
for (i = 0; i < node->xstats->nb_xstats; i++)
if (rte_strscpy(reg->xstats->xstat_desc[i], node->xstats->xstat_desc[i],
RTE_NODE_XSTAT_DESC_SIZE) < 0)
```
The allocation and loop both use `node->xstats->nb_xstats`, so the sizes should match. However, this assumes `node->xstats` is well-formed. Adding an assertion or check that `node->xstats->nb_xstats` matches the actual allocated size would make this more robust, though it may be outside the scope of this minimal fix.
**Note:** This is pre-existing code, not introduced by this patch. Mentioning for completeness, but not blocking.
---
## Info
### 1. Pre-existing Resource Leak on Error Path
**File:** lib/graph/node.c
**Location:** Lines 205-209 (context around the patch)
This is not introduced by the current patch, but the existing code has a resource leak. If `rte_strscpy()` fails inside the loop (line 212-213), the code jumps to `free:` label. However, the `free:` label only frees `reg->xstats` itself, not the `reg->xstats->xstat_desc` array that was allocated at line 205.
**Existing cleanup code** (not shown in patch but relevant):
The `free:` label likely calls `free(reg->xstats)` but does not call `free(reg->xstats->xstat_desc)` first.
**Recommended fix** (for a future patch, not blocking this one):
```c
free:
if (reg->xstats != NULL) {
free(reg->xstats->xstat_desc);
free(reg->xstats);
}
/* ... existing cleanup ... */
```
This is a separate issue from the one being fixed and should be addressed in a follow-up patch.
---
## Conclusion
**Patch verdict: ACCEPT with Info notes.**
The patch correctly fixes the immediate bug (missing `nb_xstats` copy). The warnings above are about pre-existing code quality issues that should be addressed separately, not blockers for this minimal fix.
The patch is:
- Correct in its fix
- Minimal in scope (one-line change)
- Properly tagged with Fixes: and Cc: stable
- Follows DPDK coding style
**Recommendation:** Merge this patch. Consider follow-up patches to address the resource leak on error path noted above.
More information about the test-report
mailing list