On 11.07.2025 05:50, Penny Zheng wrote:
> --- a/xen/drivers/cpufreq/cpufreq.c
> +++ b/xen/drivers/cpufreq/cpufreq.c
> @@ -64,12 +64,53 @@ LIST_HEAD_READ_MOSTLY(cpufreq_governor_list);
> /* set xen as default cpufreq */
> enum cpufreq_controller cpufreq_controller = FREQCTL_xen;
>
> -enum cpufreq_xen_opt __initdata cpufreq_xen_opts[2] = { CPUFREQ_xen,
> - CPUFREQ_none };
> +enum cpufreq_xen_opt __initdata cpufreq_xen_opts[NR_CPUFREQ_OPTS] = {
> + CPUFREQ_xen,
> + CPUFREQ_none
> +};
> unsigned int __initdata cpufreq_xen_cnt = 1;
Given this, isn't the array index 1 initializer quite pointless above? Or
else, if you really mean to explicitly fill all slots with CPUFREQ_none
(despite that deliberately having numeric value 0), why not
"[1 ... NR_CPUFREQ_OPTS - 1] = CPUFREQ_none" (or using ARRAY_SIZE(), as
per below)?
> static int __init cpufreq_cmdline_parse(const char *s, const char *e);
>
> +static bool __init cpufreq_opts_contain(enum cpufreq_xen_opt option)
> +{
> + unsigned int count = cpufreq_xen_cnt;
> +
> + while ( count-- )
> + {
> + if ( cpufreq_xen_opts[count] == option )
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static int __init handle_cpufreq_cmdline(enum cpufreq_xen_opt option)
> +{
> + int ret = 0;
> +
> + if ( cpufreq_opts_contain(option) )
> + return 0;
> +
> + cpufreq_controller = FREQCTL_xen;
> + ASSERT(cpufreq_xen_cnt < NR_CPUFREQ_OPTS);
This would better use ARRAY_SIZE(), at which point NR_CPUFREQ_OPTS can go
away again. What's worse, though, is that on release builds ...
> + cpufreq_xen_opts[cpufreq_xen_cnt++] = option;
... you then still overrun this array if something's wrong in this regard.
Jan
