On 25/03/2025 12:53 pm, Jan Beulich wrote: > There's little point in allocation two uint32_t[] arrays separately. > We'll need the bigger of the two anyway, and hence we can use that > bigger one also for transiently storing the smaller number of items. > > While there also drop j (we can use i twice) and adjust the type of > the remaining two variables on that line. > > Signed-off-by: Jan Beulich <jbeul...@suse.com>
Wow this function is a mess. It is an improvement, so Acked-by: Andrew Cooper <andrew.coop...@citrix.com>, but the allocations could be removed entirely by restructuring the logic some more. Also, one extra observation. > > --- a/xen/drivers/acpi/pmstat.c > +++ b/xen/drivers/acpi/pmstat.c > @@ -193,11 +193,10 @@ static int get_cpufreq_para(struct xen_s > const struct processor_pminfo *pmpt; > struct cpufreq_policy *policy; > uint32_t gov_num = 0; > - uint32_t *affected_cpus; > - uint32_t *scaling_available_frequencies; > + uint32_t *data; > char *scaling_available_governors; > struct list_head *pos; > - uint32_t cpu, i, j = 0; > + unsigned int cpu, i = 0; > > pmpt = processor_pminfo[op->cpuid]; > policy = per_cpu(cpufreq_cpu_policy, op->cpuid); > @@ -219,25 +218,22 @@ static int get_cpufreq_para(struct xen_s > return -EAGAIN; > } > > - if ( !(affected_cpus = xzalloc_array(uint32_t, op->u.get_para.cpu_num)) ) > + if ( !(data = xzalloc_array(uint32_t, > + max(op->u.get_para.cpu_num, > + op->u.get_para.freq_num))) ) > return -ENOMEM; > + > for_each_cpu(cpu, policy->cpus) > - affected_cpus[j++] = cpu; > + data[i++] = cpu; > ret = copy_to_guest(op->u.get_para.affected_cpus, > - affected_cpus, op->u.get_para.cpu_num); > - xfree(affected_cpus); > - if ( ret ) > - return ret; > + data, op->u.get_para.cpu_num); > > - if ( !(scaling_available_frequencies = > - xzalloc_array(uint32_t, op->u.get_para.freq_num)) ) > - return -ENOMEM; > for ( i = 0; i < op->u.get_para.freq_num; i++ ) > - scaling_available_frequencies[i] = > - pmpt->perf.states[i].core_frequency * 1000; > + data[i] = pmpt->perf.states[i].core_frequency * 1000; > ret = copy_to_guest(op->u.get_para.scaling_available_frequencies, > - scaling_available_frequencies, op->u.get_para.freq_num); > - xfree(scaling_available_frequencies); > + data, op->u.get_para.freq_num) ?: ret; > + > + xfree(data); > if ( ret ) > return ret; > Not altered by this patch, but `ret` is bogus here. It's the number of bytes not copied, and needs transforming into -EFAULT here and later. ~Andrew
