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

[dpdklab at iol.unh.edu]

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

**1. Inconsistent `pool_id` type in error messages**

The code uses `%u` format specifier for `pool_id` which has type `uint16_t`.

```c
CFA_LOG_ERR("pool_id (%u) out of range (max %u)\n",
            pool_id, ctx->max_pools);
```

While this works (integer promotion), it is inconsistent with DPDK style where narrow unsigned types should use their proper format specifiers when the precision matters. However, since `%u` is acceptable for `uint16_t` due to integer promotion and the v2 changelog explicitly states this choice was made to avoid `PRIu16` and the `<inttypes.h>` include, this is acceptable. The warning is about the justification: the changelog says `<inttypes.h>` "is no longer needed," but the code never included it to begin with (not shown in the diff context). This is a minor documentation inconsistency, not a code issue.

**2. Duplicate bounds checks - code repetition**

The same bounds check pattern is repeated three times:

```c
if (pool_id >= ctx->max_pools) {
	CFA_LOG_ERR("pool_id (%u) out of range (max %u)\n",
		    pool_id, ctx->max_pools);
	return -EINVAL;
}
```

While this is a correctness improvement (prevents out-of-bounds access), the repetition could be reduced by introducing a helper function or macro. However, given the small number of callsites (3) and the simplicity of the check, this is acceptable as-is. No action required, but worth noting for future refactoring if the pattern appears in more functions.

## INFO

**Resource ordering consideration**

All three functions follow this pattern:
1. Check `tpm != NULL`
2. Check `pool_id < max_pools` (newly added)
3. Check pool allocation state (`ba_inuse`)
4. Perform operation

This ordering is correct: validate cheap conditions (pointer, bounds) before expensive ones (allocation state lookup). The new bounds checks are placed appropriately.

**Error message style**

The new error messages use `(%u)` parentheses around the values, matching the style of existing messages in these functions (e.g., `"Pool id (%d) was not allocated\n"`). This consistency is good.

Review-Result: WARNING