|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 10:33:52 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

## Correctness Bugs

**Error: Resource leak on error path (line 713-714)**

The patch adds error handling when `tf_sram_alloc_block()` returns NULL, but the implementation leaks the first allocated RM index.

The code correctly releases both `index` and `next_index` when the block allocation fails:

```c
if (unlikely(!block)) {
	fparms.index = index;
	fparms.subtype = parms->tbl_type;
	fparms.rm_db = parms->rm_db;
	tf_rm_free(&fparms);        /* frees index */
	fparms.index = next_index;
	tf_rm_free(&fparms);         /* frees next_index */
	return -ENOMEM;
}
```

However, this cleanup path is incomplete. Looking at the broader function context (lines 687-712 visible in the diff), both `index` and `next_index` are allocated from the resource manager before this point. The error path properly releases both, which is correct.

**Error withdrawn after verification** - the cleanup is actually correct. Both allocations are released.

## Style and Process

**Info: likely() vs unlikely() consistency**

The patch uses `unlikely(!block)` (line 713). This is correct for an error path check. The use of `unlikely()` is appropriate here as allocation failure should be rare.

**Info: Error propagation**

The function returns `-ENOMEM` when block allocation fails, which is consistent with error handling patterns in DPDK and matches the comment context showing similar error returns elsewhere in the function.

## C Coding Style

The added code follows DPDK coding style:
- Proper indentation with tabs
- Braces on correct lines
- Error path uses early return pattern
- Explicit NULL comparison with `!block`

## Verification

The patch correctly addresses the bug described in the commit message:
1. Checks the NULL return from `tf_sram_alloc_block()`
2. Releases both RM indices (`index` and `next_index`) that were allocated earlier
3. Returns appropriate error code `-ENOMEM`
4. Prevents the subsequent NULL pointer dereference via `block->block_id`

The fix is correct and complete.

## Summary

The patch properly fixes the resource leak and NULL pointer dereference bug. The error handling correctly releases both allocated RM indices and returns an appropriate error code. No style or process violations identified.

Review-Result: CLEAN


More information about the test-report mailing list