[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] libxc/PM: correct (not just) error handling in xc_get_cpufreq_para()
On Thu, Mar 27, 2025 at 02:32:04PM +0100, Jan Beulich wrote: > HWP work made the cleanup of the "available governors" array > conditional, neglecting the fact that the condition used may not be the I don't know why the cleanup was made conditional, as long as the bounce buffer is declared with DECLARE_NAMED_HYPERCALL_BOUNCE(), xc_hypercall_bounce_post() can be called without issue (even if ..bounce_pre isn't used). Maybe it's all those "unlock_*" labels that is misleading, a single "out" label which does the cleanup unconditionally would be more than enough. > condition that was used to allocate the buffer (as the structure field > is updated upon getting back EAGAIN). Throughout the function, use the > local variable being introduced to address that. > > --- a/tools/libs/ctrl/xc_pm.c > +++ b/tools/libs/ctrl/xc_pm.c > @@ -212,31 +212,32 @@ int xc_get_cpufreq_para(xc_interface *xc > DECLARE_NAMED_HYPERCALL_BOUNCE(scaling_available_governors, > user_para->scaling_available_governors, > user_para->gov_num * CPUFREQ_NAME_LEN * sizeof(char), > XC_HYPERCALL_BUFFER_BOUNCE_BOTH); > - > - bool has_num = user_para->cpu_num && > - user_para->freq_num && > - user_para->gov_num; > + unsigned int in_gov_num = user_para->gov_num; > + bool has_num = user_para->cpu_num && user_para->freq_num; > > if ( has_num ) I think there's an issue here, this condition used to be true if `gov_num` was not 0, even if `cpu_num` and `freq_num` was 0. That's not the case anymore. Shouldn't `has_num` use also the value from `gov_num`? Do we actually need `has_num`? Thanks, -- Anthony Perard | Vates XCP-ng Developer XCP-ng & Xen Orchestra - Vates solutions web: https://vates.tech
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |