Re: struct mctelem_cookie missing definition

[Jan Beulich <jbeulich@xxxxxxxx>]

On 14.02.2025 08:46, Nicola Vetrini wrote:
> On 2025-02-14 04:00, Stefano Stabellini wrote:
>> On Thu, 13 Feb 2025, Stefano Stabellini wrote:
>>>>> diff --git a/xen/arch/x86/cpu/mcheck/mctelem.h 
>>>>> b/xen/arch/x86/cpu/mcheck/mctelem.h
>>>>> index f4c5ff848d..2ccd490e5d 100644
>>>>> --- a/xen/arch/x86/cpu/mcheck/mctelem.h
>>>>> +++ b/xen/arch/x86/cpu/mcheck/mctelem.h
>>>>> @@ -52,7 +52,7 @@
>>>>>   * the element from the processing list.
>>>>>   */
>>>>>
>>>>> -typedef struct mctelem_cookie *mctelem_cookie_t;
>>>>> +typedef uint64_t *mctelem_cookie_t;
>>>>
>>>> Yet that makes it possible to de-reference the pointer. Which, as Andrew
>>>> explained, is intended to be impossible. If this could be properly
>>>> replaced (not exactly what Andrew indicated by "file it in /dev/null"),
>>>> fine. Truly purging the code (i.e. as Andrew suggests) may still be an
>>>> option, with appropriate justification. But simply adjusting the type
>>>> and then moving on is too little, imo. Even if you used void * (to make
>>>> de-referencing impossible) I'd view it as largely papering over an issue;
>>>> then converting to other pointers (without explicit cast, and hence
>>>> without making apparent the badness of doing so) would become possible.
>>>
>>> What about converting to uintptr_t (not a pointer)?
>>>
>>>
>>> In general, there are quite a few MISRA rules that we could mark as
>>> blocking (clean) in our GitLab scan with just a few code changes like
>>> this one. My goal is to make these rules blocking as soon as possible.
>>> If I can improve the code in the process, that is even better, but it 
>>> is
>>> not mandatory. And I would rather spend one more hour marking a second
>>> rule as blocking instead.
>>>
>>> What I mean is that I believe it would be acceptable to make some
>>> compromises and accept non-perfect changes to the code if it helps us
>>> enforce more rules as blocking in GitLab CI.
>>
>> After briefly speaking with Andrew about this, and re-reading Jan's
>> email above, I think it is best to resolve this as a deviation.
>>
>> Would this deviation work for you? Please suggest a better wording if
>> you prefer.
>>
>> Nicola, in reality I think it would be better to use deviations.rst
>> because the SAF comment below would need to be replicated at every call
>> side, if I am not mistaken.
>>
> 
> Would deviating macros "COOKIE2MCTE" and "MCTE2COOKIE" work?

If it did, COOKIE2ID and ID2COOKIE would likely need including as well.

Jan

> In that case, that is a simple configuration tweak which then will be 
> justified in deviations.rst.
> 
> Thanks,
>   Nicola