|SUCCESS| fib6: fix signedness in common root build

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Thu Oct 22 11:00:23 CEST 2026


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

_AI Code Review_

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

# DPDK Patch Review

## Correctness Issues

**Error: Potential sign extension on line with `get_idx()` call**

The patch correctly identifies that `get_idx()` returns `uint32_t`, but the original code stored it in a signed `int idx`. The fix changes `idx` to `uint32_t`. However, there's still a problem on line 325:

```c
idx = get_idx(ip->a, i);
```

This line is not shown in the patch context, but the fix description states `get_idx()` returns `uint32_t`. The patch correctly changes `idx` to `uint32_t` on line 320. This is correct.

**Error: Missing widening cast in existing code visible in patch**

Line 342 in the new code:
```c
*tbl = get_tbl_p_by_idx(cur_tbl,
	(uint64_t)prev_idx * TRIE_TBL8_GRP_NUM_ENT, dp->nh_sz);
```

This is actually **correct** - the patch properly widens `prev_idx` to `uint64_t` before multiplication. This matches the guideline for integer multiply without widening cast.

**Review of added widening cast on line 332:**
```c
write_to_dp(tbl_ptr, ((uint64_t)tbl8_idx << 1) |
	TRIE_EXT_ENT, dp->nh_sz, 1);
```

This widening cast is correct and necessary. `tbl8_idx` is now `int` (can be negative for error codes from `tbl8_alloc()`), but when used here it's known to be non-negative. The cast to `uint64_t` before shifting prevents any sign extension issues and ensures the shift doesn't overflow if `tbl8_idx` has bit 31 set (though in practice tbl8 indices shouldn't be that large).

## Style Issues

None identified. The patch follows DPDK coding style:
- Variable declarations at the start of the block
- Appropriate use of `uint32_t` for unsigned values
- Proper widening casts before operations that could overflow
- Consistent indentation and formatting

## Documentation

**Info: Release notes entry recommended but not required**

This is a correctness bug fix for existing code. While the commit message includes `Fixes:` and `Cc: stable at dpdk.org` tags (which are correct), a release notes entry in `doc/guides/rel_notes/` would help users understand this fix addresses a sign extension bug that could produce bogus table pointers. However, per the guidelines, release notes are not strictly required for bug fixes, only for API changes and new features.

## Summary

The patch correctly addresses a sign extension bug where `get_idx()` returned `uint32_t` but was stored in `int`, causing negative values when bit 31 was set. The fix:

1. Makes `idx` and `prev_idx` unsigned (`uint32_t`)
2. Introduces a separate signed `tbl8_idx` to handle `tbl8_alloc()` error returns
3. Properly widens to `uint64_t` before shift and multiply operations

The implementation is correct and follows DPDK coding standards.

Review-Result: CLEAN


More information about the test-report mailing list