|WARNING| [v2] fib, rib: fix name prefixes

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-08-13

# DPDK Patch Review

## Summary
This patch fixes FIB/RIB library object naming conventions to avoid double-prefixing and adds name length validation. The changes improve clarity and prevent opaque failures from oversized names.

---

## Errors

### 1. Error message format string type mismatch (rte_rib.c, rte_rib6.c)

**lib/rib/rte_rib.c:431** and **lib/rib/rte_rib6.c:496**:
```c
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
    name, RTE_MEMPOOL_NAMESIZE - sizeof(RIB_MEM_PREFIX));
```

**Problem**: `sizeof(RIB_MEM_PREFIX)` returns `size_t` but computes the size of a string literal (including null terminator), not the prefix length. The arithmetic result type is correct for `%zu`, but the value is semantically wrong.

**Fix**: Use `strlen()` to get the actual prefix length, or just subtract 1 from sizeof to account for the null terminator:
```c
RIB_LOG(ERR, "RIB name %s is too long, limit is %zu characters",
    name, RTE_MEMPOOL_NAMESIZE - strlen(RIB_MEM_PREFIX) - 1);
/* or */
name, RTE_MEMPOOL_NAMESIZE - sizeof(RIB_MEM_PREFIX) + 1 - 1);
```
(The `+1` cancels the null terminator in sizeof, then `-1` for the buffer's own null terminator.)

Same issue in `rte_rib6.c:496`.

---

## Warnings

### 1. Inconsistent error message wording (rte_fib.c vs rte_fib6.c)

**lib/fib/rte_fib.c:189** vs **lib/fib/rte_fib6.c:190**:
- FIB: `"Can not allocate RIB for FIB: %s"`
- FIB6: `"Can not allocate RIB6 for FIB6 %s"`

One has a colon before `%s`, the other doesn't. Consider matching them for consistency:
```c
"Can not allocate RIB for FIB %s"
"Can not allocate RIB6 for FIB6 %s"
```

### 2. Missing type specification on snprintf length constant

**lib/rib/rte_rib.c:432**, **lib/rib/rte_rib6.c:498**:
```c
if (unlikely(ret < 0 || ret >= (int)RTE_MEMPOOL_NAMESIZE)) {
```

The cast is correct (snprintf returns `int`), but the pattern is slightly awkward. Both conditions check `ret` which is already `int`, so this is fine. However, for absolute clarity when comparing `int` return value against a size constant, some codebases prefer:
```c
if (unlikely(ret < 0 || (size_t)ret >= RTE_MEMPOOL_NAMESIZE)) {
```
This is a style preference; current code is correct.

---

## Info

### 1. Commit references two Bugzilla IDs without URLs
The commit message lists `Bugzilla ID: 1981 1982` but does not provide URLs. Per DPDK convention, if referencing multiple bugs, consider listing them on separate lines or clarifying the relationship (are both IDs the same root cause?).

### 2. Release notes formatting
**doc/guides/rel_notes/release_26_11.rst:104**:
```
  As the prefixes above are added on top of the name,the new maximum length of
```
Missing space after comma: `name,the` should be `name, the`.

### 3. Test name length verification
The patch shortens test names in `app/test/test_fib.c` and `app/test/test_fib6.c` to fit within the new limits. Consider verifying that the new names still provide sufficient context for debugging (e.g., `"test_create_inv"` is clear, but `"test_mult_crt"` loses the full word "create"). This is a readability consideration, not a functional issue.

---

## Correctness Verification

**No correctness bugs identified.** Error paths properly set `rte_errno` and return NULL. No resource leaks, use-after-free, or missing error checks detected.

**Specific checks performed:**
- `snprintf` return value checked for overflow before use 
- `rte_errno` set to `ENAMETOOLONG` on name validation failure 
- All new error paths return NULL and do not leak `node_pool` (mempool creation happens after length check) 
- Existing mempool creation failure path already frees nothing (pool itself is the first allocation) 

---

## Style Compliance

- Indentation, bracing, and naming conventions: **compliant**
- No forbidden tokens detected
- No deprecated API usage
- Prefix conventions followed (`RTE_`, `rte_`, and driver-specific where appropriate)
- Boolean comparisons: The code uses explicit NULL checks and integer comparisons correctly

---

## Documentation and API

- Release notes updated appropriately 
- Changes documented in API Changes section 
- New error condition (`ENAMETOOLONG`) is observable behavior and is documented 
- No ABI break (name length limits are input validation, not structure changes) 

---

**Recommendation**: Fix the `sizeof(RIB_MEM_PREFIX)` arithmetic in error messages (Error #1) and address the formatting typo in release notes (Info #2). Other items are minor polish.