|WARNING| [v2] net/bnxt/tf_core: fix buffer leaks in RM create DB

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Errors

**Use of forbidden `return` instead of `goto` in error path (line 507)**

Line 507:
```c
if (rc)
    return rc;
```

The function now has a common error cleanup path (`free_bufs:`) that must release all allocated buffers. Early returns bypass this cleanup, causing resource leaks when errors occur after `query` or `req_cnt` have been allocated but before other buffers are allocated. This specific path leaks `query` if the `req_cnt` allocation fails.

The patch already converts most early returns to `goto free_bufs`, but line 507 was missed. Change to:
```c
if (rc)
    goto free_bufs;
```

---

## Warnings

None.

---

## Info

**Pre-existing: inconsistent NULL-initialization pattern**

The patch initializes several pointer variables to NULL at declaration (lines 455-459 in the patch context). This is good practice for the cleanup path pattern the patch introduces. However, other local variables in the same function (`i`, `j`, `max_types`, `hcapi_items`, `pool_size`) are not initialized where they arguably could be, and `tfs`, `dev`, `db` remain uninitialized. This inconsistency is pre-existing -- the patch only touches the pointers it needs for cleanup -- but future work could consider initializing all locals where a meaningful initial value exists, or none of them, for consistency within the function.

Not introduced by this patch; no action required here.

---

## Summary

The patch correctly introduces a common cleanup path (`free_bufs:`) and converts most early error returns to use it. This fixes the resource leaks described in the commit message. However, one early return at line 507 was missed and still bypasses cleanup, leaking the `query` buffer when `req_cnt` allocation fails.

Review-Result: ERROR