|WARNING| ethdev/telemetry: fix hide_zero parameter being reset

[dpdklab at iol.unh.edu]

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

_AI Code Review_

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

Thank you for submitting this patch for review. I'll analyze it against the DPDK coding standards and contribution guidelines.

## ANALYSIS

This patch fixes a logic error in `lib/ethdev/rte_ethdev_telemetry.c` where the `hide_zero` parameter is unconditionally reset to `false` after being successfully parsed, rendering the feature non-functional.

### ERRORS

None identified.

### WARNINGS

**1. Missing tests**

The patch fixes a bug in telemetry command parameter handling, but provides no test coverage. The ethdev telemetry layer should have functional tests in `app/test` that verify:
- The `hide_zero` parameter is correctly parsed and passed through
- The filter value is handled properly
- Error paths (NULL kvlist, empty filter_val, parse failure) behave as expected

Without tests, this bug could regress undetected in the future.

### CORRECTNESS REVIEW

The fix is correct. The original code path was:

```c
ret = rte_kvargs_process(kvlist, "hide_zero", eth_dev_parse_hide_zero, &hide_zero);
if (kvlist == NULL || (filter_val != NULL && strlen(filter_val) == 0) || ret != 0) {
    /* error path */
} else {
    filter = (filter_val != NULL) ? strdup(filter_val) : NULL;
    hide_zero = false;  /* BUG: unconditionally resets parsed value */
}
```

The patched code correctly removes the reset:

```c
if (kvlist == NULL || (filter_val != NULL && strlen(filter_val) == 0) || ret != 0)
    /* error path */
else
    filter = (filter_val != NULL) ? strdup(filter_val) : NULL;
    /* hide_zero retains the parsed value from rte_kvargs_process */
```

**Error path verification:**
- The function returns early with `-EINVAL` on error conditions, so `filter` and `hide_zero` are not used on the error path
- No resource leak: `rte_kvargs_free(kvlist)` is called unconditionally after the if/else block

**No use-after-free risk:**
- `hide_zero` is a local `bool` passed by reference to the parser
- `filter` is passed to `eth_dev_get_port_regs()`, which is responsible for freeing it (not shown in this patch but documented in the original commit ba4d57bd849d)

### STYLE REVIEW

The code follows DPDK style:
- Proper indentation
- Explicit comparison against `NULL` and `0`
- Single-statement if/else branches without braces (acceptable per guidelines)

### DOCUMENTATION

The commit message clearly describes:
- The bug (hide_zero reset in else branch)
- The impact (feature silently ignored)
- The fix (remove the reset)

No release notes required per guidelines - this is a bug fix to existing functionality, not a new feature or API change.

## SUMMARY

The patch correctly fixes a logic bug where a successfully parsed parameter was being discarded. The code is correct, follows style guidelines, and has a clear commit message.

The only weakness is the absence of test coverage, which would prevent this regression from occurring again.

Review-Result: WARNING