|WARNING| [v2] net/bnxt: validate pool index in pool manager APIs

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Oct 13 08:23:50 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None identified.

## WARNINGS

**W1: Inconsistent format specifier usage**

The patch adds three identical error messages using `%u` for `pool_id` and `ctx->max_pools`:
```c
CFA_LOG_ERR("pool_id (%u) out of range (max %u)\n",
            pool_id, ctx->max_pools);
```

However, the existing code in this file uses `%d` for similar identifiers:
```c
CFA_LOG_ERR("Pool id (%d) was not allocated\n", pool_id);
CFA_LOG_ERR("A function (%d) is still using the pool (%d)\n",
            ctx->fid_tbl[pool_id], pool_id);
```

**Why it matters:** Mixing `%d` and `%u` for the same types (`uint16_t pool_id`) in the same error message family is inconsistent and could confuse readers or cause confusion if values near UINT16_MAX are involved.

**Suggested fix:** Use `%u` consistently for all `pool_id` format specifiers in this file, or use `%d` consistently. Since `pool_id` is `uint16_t` (unsigned), `%u` is technically more correct. Consider updating the existing error messages to match, or align the new messages with the existing style.

## INFO

**I1: Bounds check placement**

The bounds check `pool_id >= ctx->max_pools` is added before the allocation check `!ba_inuse(ctx->pool_ba, pool_id)` in `cfa_tpm_fid_add()` and `cfa_tpm_fid_rem()`. This is the correct order - it prevents a potential out-of-bounds access to `ctx->fid_tbl[pool_id]` in the error message path of the subsequent check.

**I2: Error message quality**

The error messages clearly identify both the invalid value and the valid range, which aids debugging.

---

Review-Result: WARNING


More information about the test-report mailing list