|WARNING| [v2] test: defer timer initialization

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

# DPDK Patch Review

## ERRORS

None.

## WARNINGS

**test_pdcp.c - Timer initialization error path cleanup incomplete**

In `test_expiry_with_rte_timer()`, after adding `rte_timer_subsystem_init()` at line 1762, the function can return `TEST_FAILED` at line 1765 without stopping or cleaning up the rte_timer that will be initialized later. If the timer subsystem init succeeds but a subsequent allocation fails, the initialized timer subsystem may be left in an inconsistent state if other tests don't expect it to be initialized.

However, per the guidelines, `rte_timer_subsystem_init()` returning `-EALREADY` is explicitly handled, suggesting the subsystem is designed to be idempotent and multiple initializations are acceptable. This reduces the severity from Error to Warning.

**Suggested fix:**
This is likely acceptable as-is given the `-EALREADY` handling pattern used throughout. If cleanup is required, it should be documented in the timer API. No change needed unless timer API documentation specifies cleanup requirements.

---

**test_timer_secondary.c - Code style: missing braces on multi-line if**

Line 130-133: The condition check was refactored, but the new multi-line if statement lacks braces:

```c
if (proc_type == RTE_PROC_PRIMARY && rte_lcore_count() < NUM_LCORES_NEEDED) {
    printf("Not enough cores for test_timer_secondary, expecting at least %u\n",
           NUM_LCORES_NEEDED);
    return TEST_SKIPPED;
}
```

The body spans multiple lines (printf call continues to next line), which per DPDK style guidelines should use braces. The code has braces, so this is actually correct. Disregard this observation.

---

**test_timer_secondary.c - Minor: condition simplification changes semantics slightly**

The original code at lines 130-135 had nested structure:
```c
if (proc_type == RTE_PROC_PRIMARY) {
    if (rte_lcore_count() < NUM_LCORES_NEEDED) {
```

Changed to (lines 130-133):
```c
if (proc_type == RTE_PROC_PRIMARY && rte_lcore_count() < NUM_LCORES_NEEDED) {
```

This is a logical refactoring that maintains the same behavior. The timer init was moved after this check (line 136), so it now runs for both PRIMARY and SECONDARY processes. This appears intentional since timer init is needed in both process types. No issue here.

---

## INFO

**Timer subsystem initialization pattern is consistent**

The patch adds the same initialization pattern across all modified test files:
```c
ret = rte_timer_subsystem_init();
if (ret < 0 && ret != -EALREADY) {
    printf("timer subsystem init failed: %d\n", ret);
    return TEST_FAILED;
}
```

This is a good consistent pattern that handles both first initialization and repeated calls. The `-EALREADY` handling indicates the subsystem is designed to be idempotent.

**Scope reduction is beneficial**

Moving timer initialization from global `main()` to individual test functions that need it is good practice. This reduces resource usage for tests that don't need timers and makes dependencies explicit.

Review-Result: WARNING