|WARNING| [v2,3/3] vhost: remove use of strncpy

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## Patch 1/3: ethdev: remove use of strncpy

### Errors

**Bounds check logic error:**
The condition `(len - 2) >= BUFSIZ` is incorrect for detecting overflow.
If `len - 2` equals `BUFSIZ`, the subsequent `memcpy(str, &p_val[1], len - 2)` writes exactly `BUFSIZ` bytes into a `BUFSIZ`-sized buffer, leaving no room for the null terminator that the later string operations expect.
The check should be `(len - 2) >= (BUFSIZ - 1)` or `(len - 2) > (BUFSIZ - 1)` to ensure space for a null byte.

```c
/* Current code - allows exactly BUFSIZ bytes, no room for null */
if (len < 2 || (len - 2) >= BUFSIZ) {

/* Should be - ensures room for null terminator */
if (len < 2 || (len - 2) >= (BUFSIZ - 1)) {
```

The existing `memset(str, 0, BUFSIZ)` before the copy does provide a trailing null at `str[len-2]` if `len-2 == BUFSIZ-1`, but if `len-2 == BUFSIZ`, the null is at index `BUFSIZ` which is out of bounds. Given the subsequent code uses `str[i]` in a while loop checking for `'\0'`, an exact-size copy could walk off the end of the buffer if the input is malformed.

### Info

The change from `strncpy()` to `memcpy()` is appropriate here since the code is explicitly not dealing with null-terminated strings (it strips the outer `[` `]` and processes the interior). The added bounds check is a good defensive measure.

---

## Patch 2/3: eventdev: improve bounds checks for names in adapter create

### No Issues Found

The changes correctly replace the hardcoded `TXA_MEM_NAME_LEN` / `TXA_SERVICE_NAME_LEN` defines with `sizeof()`, and replace `strncpy()` with `strlcpy()`. Both strings are null-terminated C strings here, so `strlcpy()` is the correct choice.

---

## Patch 3/3: vhost: remove use of strncpy

### Warnings

**`vhost_set_ifname()` signature change without documentation:**
The function signature changes from `vhost_set_ifname(int vid, const char *if_name, unsigned int if_len)` to `vhost_set_ifname(int vid, const char *if_name)`, but `vhost_set_ifname()` is declared in `vhost.h` which is an internal header (not installed). This is acceptable for internal API, but the change should be noted if this function is part of any internal interface contract.

The removal of the `if_len` parameter is safe because the only caller (`socket.c`) was computing the length solely to pass it to this function, and `strlcpy()` handles the bounds internally.

### Info

In `rte_vhost_get_ifname()`, the original code pattern:
```c
len = RTE_MIN(len, sizeof(dev->ifname));
strncpy(buf, dev->ifname, len);
buf[len - 1] = '\0';
```
is replaced with:
```c
len = RTE_MIN(len, sizeof(dev->ifname));
strlcpy(buf, dev->ifname, len);
```

This is correct. `strlcpy()` guarantees null termination, and the `RTE_MIN()` ensures the size passed to `strlcpy()` does not exceed the buffer size. The pattern is safe.

---

## Summary

### Patch 1/3:
- **Error:** Bounds check allows exactly `BUFSIZ` bytes which leaves no room for the null terminator expected by subsequent string operations. Change `>= BUFSIZ` to `>= (BUFSIZ - 1)` or `> (BUFSIZ - 1)`.

### Patch 2/3:
- No issues.

### Patch 3/3:
- **Warning:** Internal API signature change (`vhost_set_ifname`) is acceptable for internal headers but should be verified against any internal contracts.
- Code changes are correct.