|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 format specifier types**

The patch adds log messages using `%u` for `uint16_t pool_id` and `ctx->max_pools`.
While this works (uint16_t promotes to unsigned int), DPDK code typically uses sized format specifiers for explicitly sized types.

The original error messages in the file use `%d` for `pool_id` and `fid`:
```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", ...);
```

For consistency within this file, the new messages should either:
- Use `%d` to match existing style (accepts the int promotion), or
- Convert all pool_id/fid logging to `PRIu16` for type safety

The v2 commit message states that `%u` was chosen instead of `PRIu16` to avoid needing `<inttypes.h>`.
However, `cfa_tpm.c` already uses other format macros and the file includes `rte_common.h` which provides these.
The inconsistency between `%u` in new code and `%d` in existing code for the same types reduces readability.

**Suggested fix:** Match the existing style in this file by using `%d`, or update all pool_id logging to `PRIu16` consistently.

---

## Info

**1. Validation placement**

The three new bounds checks follow the same pattern:
check if `tpm` is NULL, then check if `pool_id >= ctx->max_pools`.

This ordering is correct (NULL check first prevents crash in the bounds check).
The checks are placed before any use of `pool_id` as an array index into `ctx->fid_tbl[]` and `ctx->pool_ba[]`, which prevents out-of-bounds access.

**2. Error message clarity**

The error messages are clear and include both the invalid value and the valid range, which is good practice for debugging.

---

Review-Result: WARNING