On 05/05/2021 10:16, Roger Pau Monné wrote: > On Tue, May 04, 2021 at 07:53:22PM +0100, Andrew Cooper wrote: >> It is bad form in C, perhaps best demonstrated by trying to read >> xc_cpu_policy_destroy(), and causes const qualification to have >> less-than-obvious behaviour (the hidden pointer becomes const, not the thing >> it points at). > Would this also affect cpuid_leaf_buffer_t and msr_entry_buffer_t > which hide an array behind a typedef?
They're a total pain because in userspace, they're plain arrays, and in Xen, they're GUEST_HANDLE's. Hiding arrays in a typedef like that (unlike hiding pointers) doesn't change the interaction with const. So the code there is correct AFAICT, even if it doesn't appear so. >> xc_cpu_policy_set_domain() needs to drop its (now normal) const >> qualification, >> as the policy object is modified by the serialisation operation. >> >> This also shows up a problem with the x86_cpu_policies_are_compatible(), >> where >> the intermediate pointers are non-const. >> >> Signed-off-by: Andrew Cooper <[email protected]> > Acked-by: Roger Pau Monné <[email protected]> Thanks. >> --- >> CC: Jan Beulich <[email protected]> >> CC: Roger Pau Monné <[email protected]> >> CC: Wei Liu <[email protected]> >> >> Discovered while trying to start the integration into XenServer. This wants >> fixing ASAP, before futher uses get added. >> >> Unsure what to do about x86_cpu_policies_are_compatible(). It would be nice >> to have xc_cpu_policy_is_compatible() sensibly const'd, but maybe that means >> we need a struct const_cpu_policy and that smells like it is spiralling out >> of >> control. > Not sure TBH, I cannot think of any alternative right now, but > introducing a const_cpu_policy feels kind of code duplication. At least this is all internals. We've got time and flexibility to experiment. ~Andrew
