Re: [PATCH v8 04/15] xen/sysctl: Nest cpufreq scaling options

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On Mon, Aug 07, 2023 at 02:51:08PM -0400, Jason Andryuk wrote:
> diff --git a/tools/libs/ctrl/xc_pm.c b/tools/libs/ctrl/xc_pm.c
> index c3a9864bf7..5ec050982a 100644
> --- a/tools/libs/ctrl/xc_pm.c
> +++ b/tools/libs/ctrl/xc_pm.c
> @@ -245,6 +245,45 @@ int xc_get_cpufreq_para(xc_interface *xch, int cpuid,
>      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));

These could have been done by defining a temporary macro to avoid
repeating the fields name twice, but this sanity check is good and
should prevent anyone from changing one struct without changing the
other one.

Thanks.

Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>

Cheers,

-- 
Anthony PERARD