Re: [PATCH v4 03/10] libx86: introduce helper to fetch msr entry

[Jan Beulich <jbeulich@suse.com>]

On 18.05.2021 12:50, Roger Pau Monné wrote:
> On Mon, May 17, 2021 at 05:43:33PM +0200, Jan Beulich wrote:
>> On 07.05.2021 13:04, Roger Pau Monne wrote:
>>> @@ -91,6 +91,21 @@ int x86_msr_copy_from_buffer(struct msr_policy *policy,
>>>                               const msr_entry_buffer_t msrs, uint32_t nr_entries,
>>>                               uint32_t *err_msr);
>>>  
>>> +/**
>>> + * Get a MSR entry from a policy object.
>>> + *
>>> + * @param policy      The msr_policy object.
>>> + * @param idx         The index.
>>> + * @returns a pointer to the requested leaf or NULL in case of error.
>>> + *
>>> + * Do not call this function directly and instead use x86_msr_get_entry that
>>> + * will deal with both const and non-const policies returning a pointer with
>>> + * constness matching that of the input.
>>> + */
>>> +const uint64_t *_x86_msr_get_entry(const struct msr_policy *policy,
>>> +                                   uint32_t idx);
>>> +#define x86_msr_get_entry(p, i) \
>>> +    ((__typeof__(&(p)->platform_info.raw))_x86_msr_get_entry(p, i))
>>>  #endif /* !XEN_LIB_X86_MSR_H */
>>
>> Just two nits: I think it would be nice to retain a blank line ahead of
>> the #endif. And here as well as in the CPUID counterpart you introduce,
>> strictly speaking, name space violations (via the leading underscore).
> 
> I guess another option would be to name the function
> x86_msr_get_entry_const, and keep the x86_msr_get_entry macro as-is.
> 
> Does that seem better?

This would be fine with me, as would be a trailing underscore or a
double underscore after e.g. the first name component.

Jan