[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: struct mctelem_cookie missing definition
On 14.02.2025 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. Sounds reasonable to me; one nit below. > 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. If replication indeed would be needed, I agree doing it the other way might be better. > --- a/docs/misra/safe.json > +++ b/docs/misra/safe.json > @@ -92,6 +92,14 @@ > }, > { > "id": "SAF-11-safe", > + "analyser": { > + "eclair": "MC3A2.R11.2" > + }, > + "name": "Rule 11.2: purposely impossible to dereference", > + "text": "Certain pointers points to incomplete types purposely > so that they are impossible to dereference." Nit: s/ points / point / Jan
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |