[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] libxc: remove / adjust xc_get_cpufreq_para()'s BUILD_BUG_ON()s
On Wed, Aug 23, 2023 at 9:47 AM Jan Beulich <jbeulich@xxxxxxxx> wrote: > > The full structures cannot match in layout, as soon as a 32-bit tool > stack build comes into play. But it also doesn't need to; the part of > the layouts that needs to match is merely the union that we memcpy() > from the sysctl structure to the xc one. Keep (in adjusted form) only > the relevant ones. > > Since the whole block needs touching anyway, move it closer to the > respective memcpy() and use a wrapper macro to limit verbosity. > > Fixes: 2381dfab083f ("xen/sysctl: Nest cpufreq scaling options") > Reported-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> > Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx> > > --- a/tools/libs/ctrl/xc_pm.c > +++ b/tools/libs/ctrl/xc_pm.c > @@ -248,45 +248,6 @@ int xc_get_cpufreq_para(xc_interface *xc > sys_para->freq_num = user_para->freq_num; > sys_para->gov_num = user_para->gov_num; > > - /* Sanity check struct layout */ > - BUILD_BUG_ON(sizeof(*user_para) != sizeof(*sys_para)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), cpu_num) != > - offsetof(typeof(*sys_para), cpu_num)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), freq_num) != > - offsetof(typeof(*sys_para), freq_num)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), gov_num) != > - offsetof(typeof(*sys_para), gov_num)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), affected_cpus) != > - offsetof(typeof(*sys_para), affected_cpus)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_frequencies) > != > - offsetof(typeof(*sys_para), > scaling_available_frequencies)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_available_governors) != > - offsetof(typeof(*sys_para), scaling_available_governors)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), scaling_driver) != > - offsetof(typeof(*sys_para), scaling_driver)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_cur_freq) != > - offsetof(typeof(*sys_para), cpuinfo_cur_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_max_freq) != > - offsetof(typeof(*sys_para), cpuinfo_max_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), cpuinfo_min_freq) != > - offsetof(typeof(*sys_para), cpuinfo_min_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_cur_freq) != > - offsetof(typeof(*sys_para), u.s.scaling_cur_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_governor) != > - offsetof(typeof(*sys_para), u.s.scaling_governor)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_max_freq) != > - offsetof(typeof(*sys_para), u.s.scaling_max_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.scaling_min_freq) != > - offsetof(typeof(*sys_para), u.s.scaling_min_freq)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.userspace) != > - offsetof(typeof(*sys_para), u.s.u.userspace)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.s.u.ondemand) != > - offsetof(typeof(*sys_para), u.s.u.ondemand)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), u.cppc_para) != > - offsetof(typeof(*sys_para), u.cppc_para)); > - BUILD_BUG_ON(offsetof(typeof(*user_para), turbo_enabled) != > - offsetof(typeof(*sys_para), turbo_enabled)); > - > ret = xc_sysctl(xch, &sysctl); > if ( ret ) > { > @@ -316,6 +277,22 @@ int xc_get_cpufreq_para(xc_interface *xc > BUILD_BUG_ON(sizeof(((struct xc_get_cpufreq_para *)0)->u) != > sizeof(((struct xen_get_cpufreq_para *)0)->u)); This check... > + /* Sanity check layout of the union subject to memcpy() below. */ > + BUILD_BUG_ON(sizeof(user_para->u) != sizeof(sys_para->u)); And this check are the same? Your newer one is nicer, so maybe drop the first? > +#define CHK_FIELD(fld) \ > + BUILD_BUG_ON(offsetof(typeof(user_para->u), fld) != \ > + offsetof(typeof(sys_para->u), fld)) > + > + CHK_FIELD(s.scaling_cur_freq); > + CHK_FIELD(s.scaling_governor); > + CHK_FIELD(s.scaling_max_freq); > + CHK_FIELD(s.scaling_min_freq); > + CHK_FIELD(s.u.userspace); > + CHK_FIELD(s.u.ondemand); > + CHK_FIELD(cppc_para); > + > +#undef CHK_FIELD > + > memcpy(&user_para->u, &sys_para->u, sizeof(sys_para->u)); > } > Reviewed-by: Jason Andryuk <jandryuk@xxxxxxxxx> Sorry about the breakage. I started looking at this locally, but you beat me. Thanks, Jason
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |