|SUCCESS| power/amd_pstate: fix frequency matching for continuous scaling

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162765

_AI Code Review_

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

git sha: series.patch
Author: Stephen Hemminger <stephen at networkplumber.org>
power/amd_pstate: fix frequency matching for continuous scaling

This commit fixes frequency matching in the AMD pstate driver for
continuous frequency scaling, replacing exact match with nearest-frequency
search and fixing several issues in power_init_for_setting_freq().

>  static int
>  power_init_for_setting_freq(struct amd_pstate_power_info *pi)
>  {
> -	FILE *f = NULL;
> +	FILE *f;
>  	char buf[BUFSIZ];
> -	uint32_t i, freq;
> +	char *endptr;
> +	unsigned long freq, freq_conv;
> +	unsigned long best_diff, diff;
> +	uint32_t i, best_idx;
>  	int ret;

Removing the initialization of 'f' to NULL creates a potential issue.  If
open_core_sysfs_file() fails and does not modify 'f', the error path at
the end may attempt to fclose() an uninitialized pointer.  Can this
variable remain initialized to NULL?

>  	open_core_sysfs_file(&f, "rw+", POWER_SYSFILE_SETSPEED, pi->lcore_id);
>  	if (f == NULL) {
> @@ -308,7 +315,6 @@ power_init_for_setting_freq(struct amd_pstate_power_info *pi)
>  				POWER_SYSFILE_SETSPEED);
>  		goto err;
>  	}

[ ... ]

> -	freq = strtoul(buf, NULL, POWER_CONVERT_TO_DECIMAL);
> +	errno = 0;
> +	freq = strtoul(buf, &endptr, POWER_CONVERT_TO_DECIMAL);
> +	if (errno != 0 || endptr == buf || freq == 0) {
> +		POWER_LOG(ERR, "Failed to parse frequency '%s' for lcore %u",
> +				buf, pi->lcore_id);
> +		goto err;
> +	}

The condition 'freq == 0' in the error check is questionable.  Is a
frequency of zero always invalid in this context, or is it a valid return
value from strtoul()?  Could this reject legitimate zero frequencies that
might exist on some systems?

[ ... ]

> +	best_idx = 0;
> +	best_diff = abs_diff(freq_conv, pi->freqs[0]);
> +
> +	for (i = 1; i < pi->nb_freqs; i++) {
> +		diff = abs_diff(freq_conv, pi->freqs[i]);
> +		if (diff < best_diff) {
> +			best_diff = diff;
> +			best_idx = i;
> +		}
> +	}

This nearest-frequency search assumes pi->nb_freqs > 0.  If nb_freqs is
zero, can this code access pi->freqs[0] out of bounds in the initialization
of best_diff?