|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