|SUCCESS| [v2] net/bnxt/tf_core: fix SRAM block allocation failure handling

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Tue Oct 20 09:50:27 CEST 2026


Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/171726

_AI Code Review_

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

# DPDK Patch Review

## Errors

### 1. Resource leak on error path - RM indices not released in first allocation failure case

**Line 710 and earlier in the function (not shown in patch):**

The patch adds cleanup for the case where `tf_sram_alloc_block()` returns NULL after both RM indices are allocated. However, reviewing the visible code structure reveals this is the *second* allocation failure path.

The first `tf_rm_allocate()` call (for `index`) succeeds, then the second call (for `next_index`) fails. The existing code path at that point does:

```c
fparms.index = index;
fparms.subtype = parms->tbl_type;
fparms.rm_db = parms->rm_db;
tf_rm_free(&fparms);
```

But the patch shows a nearly identical block being added for the `tf_sram_alloc_block()` failure. This means the first failure path (second RM allocation fails) likely **also** has cleanup code that the patch is duplicating.

**However, looking at the commit message:** "as is already done when the two indices are not sequential" - this confirms there IS existing cleanup for the sequential-check failure case.

The actual bug: when `tf_sram_alloc_block()` returns NULL, BOTH indices must be freed. The patch does this correctly:

```c
fparms.index = index;
/* ... */
tf_rm_free(&fparms);
fparms.index = next_index;
tf_rm_free(&fparms);
```

This is correct. The NULL dereference would occur at the line after this block (not shown in patch) where `block->block_id` is accessed.

**Severity: Error** - The patch correctly fixes a resource leak (both RM indices) and NULL pointer dereference.

**Actually, re-reading: This is a FIX, not a new bug.** The patch correctly addresses both issues. No error to report here.

## Warnings

None identified.

## Info

None.

---

## Final Checks

### Pass 1: Correctness bugs
- Verified the patch fixes a resource leak (RM indices not freed)
- Verified the patch prevents NULL pointer dereference
- This is a bug fix, not introducing a bug

### Pass 2: Style/process false positives
No style items flagged.

### Pass 3: Evidence verification
The commit message states the problem clearly, and the fix matches the description. The patch adds the missing NULL check and dual cleanup that mirrors the existing sequential-check failure path.

The patch correctly:
1. Checks the result of `tf_sram_alloc_block()` for NULL
2. Frees both RM indices (`index` and `next_index`) when allocation fails
3. Returns `-ENOMEM` to propagate the error

This is a correct bug fix.

Review-Result: CLEAN


More information about the test-report mailing list