|WARNING| app: remove use of strncpy

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-06-23

# DPDK Patch Review

## Summary
This patch replaces `strncpy()` with `strlcpy()` or `memcpy()` throughout the `app/` directory. Overall, the changes are correct and improve code safety. I identified one correctness issue and several observations about the patch.

---

## Errors

### 1. Missing null terminator in parse_string0()
**File:** `app/test-pmd/cmdline_flow.c`
**Location:** `parse_string0()` function

The replacement of `strncpy()` with `memcpy()` removes the implicit null-termination that `strncpy()` would have provided (when the source string is shorter than the destination buffer). The code does not add an explicit null terminator after the `memcpy()`.

```c
/* Current code */
memcpy(buf, str, len);
```

**Why it matters:** If `buf` is expected to be a null-terminated C string (which is likely given the function name `parse_string0`), this will leave the string unterminated. This could lead to buffer overruns in subsequent `strlen()` or string comparison operations.

**Suggested fix:**
```c
memcpy(buf, str, len);
buf[len] = '\0';  /* Add explicit null termination */
```

Or verify that the buffer is pre-zeroed and `len` accounts for space for the terminator.

---

## Warnings

### 1. args_parse() length check logic change
**File:** `app/test-regex/main.c`
**Location:** `ARG_RULES_FILE_NAME` and `ARG_DATA_FILE_NAME` cases

The original code checked `strnlen() == MAX_FILE_NAME` (indicating the string is exactly MAX_FILE_NAME-1 characters long, which is the maximum that fits with a null terminator). The new code checks `strlcpy() >= MAX_FILE_NAME` (indicating truncation occurred).

While the new logic is correct and more idiomatic for `strlcpy()`, the error messages say "to long" when they should say "too long" (typo was in the original code but worth noting if fixing nearby text).

**Note:** This is not a functional issue - both approaches correctly reject overlong strings. The `strlcpy()` approach is actually clearer.

---

## Info

### 1. Consistent improvement across multiple files
The patch correctly replaces `strncpy()` with `strlcpy()` in configuration parsing code where null-terminated strings are expected:
- `cperf_options_parsing.c`: Both changes use `strlcpy()` correctly with `sizeof(destination)`
- `main.c` (test-dma-perf): Uses `strlcpy()` correctly with `sizeof(input)`
- `evt_options.c`: Uses `strlcpy()` correctly with `sizeof(opt->test_name)`

### 2. Simplified length checking in args_parse()
The new `strlcpy()` return value check is cleaner than the previous `strnlen()` + `strncpy()` pattern. This eliminates the temporary `len` variable.

### 3. Removal of redundant null termination
**File:** `app/test-crypto-perf/cperf_options_parsing.c`
**Location:** `parse_device_type()`

The original code had:
```c
strncpy(opts->device_type, arg, sizeof(opts->device_type) - 1);
*(opts->device_type + sizeof(opts->device_type) - 1) = '\0';
```

The new code simplifies this to:
```c
strlcpy(opts->device_type, arg, sizeof(opts->device_type));
```

This is correct because `strlcpy()` always null-terminates (unlike `strncpy()`).

---

## Correctness Verification

### Resource management
- No resource allocations or deallocations in this patch
- No error paths introduced or modified
- No memory leaks possible from these changes

### Synchronization
- No shared variables modified
- No threading concerns

### Buffer safety
All `strlcpy()` replacements correctly pass `sizeof(destination)` as the size argument, ensuring buffers cannot be overrun.

---

## Conclusion

The patch is nearly correct and represents a good cleanup of unsafe `strncpy()` usage. The **one error** that must be fixed is in `parse_string0()` where `memcpy()` leaves the string unterminated. All other changes are improvements over the original code.