|WARNING| net/ixgbe/base: fix string truncation warning in ACI debug dump

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Oct 26 23:03:30 CET 2026


Test-Label: ai-code-review
Test-Status: WARNING
https://dpdk.org/patch/172132

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-10-26

# DPDK Patch Review: net/ixgbe/base: fix string truncation warning in ACI debug dump

## ERRORS

**Resource Leak on Error Path**
Line 48 in new code:
```c
if (!hw || !buf || !row_size)
    return;
```
This patch does not introduce a resource leak -- the early return on invalid parameters is correct and no resources have been allocated prior to this check. However, examining the modified function reveals a potential issue: the loop at line 52 could produce a buffer overflow if `snprintf` returns a value >= `sizeof(line)` but the code continues to call `snprintf` at `line + len` without verifying `len < sizeof(line)` first.

Line 56-58:
```c
len += snprintf(line + len, sizeof(line) - len,
                "0x%02X ", buf[i + j]);
```
If the first `snprintf` at line 54 returns `len >= sizeof(line)` (truncation occurred), the subsequent `snprintf` at line 56 is called with `line + len` where `len` may be greater than or equal to `sizeof(line)`. This results in `sizeof(line) - len` wrapping to a huge `size_t` value (since `size_t` is unsigned), causing `snprintf` to write beyond the buffer bounds.

**Suggested fix:** Check `len < sizeof(line)` after each `snprintf`:
```c
len = snprintf(line, sizeof(line), "0x%04X : ", i);
if (len >= sizeof(line))
    break;
for (j = 0; j < row_size && i + j < buf_size; j++) {
    len += snprintf(line + len, sizeof(line) - len,
                    "0x%02X ", buf[i + j]);
    if (len >= sizeof(line))
        break;
}
```

**Integer Type Mismatch**
Line 45:
```c
u32 i, j;
```
The loop variables `i` and `j` are declared as `u32`, but `buf_size` and `row_size` are `u16` (from the function signature). While this is not wrong per se, using `u16` for the loop counters would be more consistent and avoid implicit widening conversions. This is a minor point and may be intentional for performance, but worth noting.

## WARNINGS

**Inconsistent Loop Bounds Check**
Line 55:
```c
if (len >= sizeof(line))
    break;
```
This check appears after the first `snprintf` but not after the second one inside the inner loop (line 56-58). Either both should check, or the code should verify that truncation cannot occur. The current pattern checks once but then continues appending without rechecking.

**Missing Bounds Verification for snprintf Return**
Line 54 and 56-58: The code does not verify that `snprintf` succeeded or that the return value is reasonable. `snprintf` can return -1 on encoding error, though this is rare in practice. Checking `len < 0` after each call would be defensive.

## Review-Result: ERROR


More information about the test-report mailing list