On 22.06.2022 16:27, Bertrand Marquis wrote: > Hi, > >> On 22 Jun 2022, at 15:10, Jan Beulich <jbeul...@suse.com> wrote: >> >> On 22.06.2022 15:55, Bertrand Marquis wrote: >>> Hi Jan, >>> >>>> On 22 Jun 2022, at 14:01, Jan Beulich <jbeul...@suse.com> wrote: >>>> >>>> On 22.06.2022 14:55, Michal Orzel wrote: >>>>> On 22.06.2022 12:25, Jan Beulich wrote: >>>>>> On 20.06.2022 09:02, Michal Orzel wrote: >>>>>>> This series fixes all the findings for MISRA C 2012 8.1 rule, reported >>>>>>> by >>>>>>> cppcheck 2.7 with misra addon, for Arm (arm32/arm64 - target >>>>>>> allyesconfig). >>>>>>> Fixing this rule comes down to replacing implicit 'unsigned' with >>>>>>> explicit >>>>>>> 'unsigned int' type as there are no other violations being part of that >>>>>>> rule >>>>>>> in the Xen codebase. >>>>>> >>>>>> I'm puzzled, I have to admit. While I agree with all the examples in the >>>>>> doc, I notice that there's no instance of "signed" or "unsigned" there. >>>>>> Which matches my understanding that "unsigned" and "signed" on their own >>>>>> (just like "long") are proper types, and hence the omission of "int" >>>>>> there is not an "omission of an explicit type". >>>>>> >>>>> Cppcheck was choosed as a tool for MISRA checking and it is considering >>>>> it as a violation. >>>> >>>> Which by no means indicates that the tool pointing out something as a >>>> violation actually is one. >>>> >>>>> It treats unsigned as an implicit type. You can see this flag in cppcheck >>>>> source code: >>>>> >>>>> "fIsImplicitInt = (1U << 31), // Is "int" token implicitly added?" >>>> >>>> Neither the name of the variable nor the comment clarify that this is about >>>> the specific case of "unsigned". As said there's also the fact that they >>>> don't appear to point out the lack of "int" when seeing plain "long" (or >>>> "long long"). I fully agree that "extern x;" or "const y;" lack explicit >>>> "int". >>> >>> I am a bit puzzled here trying to understand what you actually want here. >>> >>> Do you suggest the change is ok but you are not ok with the fact that is >>> flagged >>> as MISRA fix even though cppcheck is saying otherwise ? >> >> First of all I'd like to understand whether what we're talking about here >> are actually violations (and if so, why that is). Further actions / requests >> depend on the answer. > > I would say yes but this would need to be confirmed by Roberto I guess. > In any case if we think this is something we want and the change is right > and cppcheck is saying that it solves a violation, I am wondering what is > the point of discussing that extensively this change.
Because imo a patch shouldn't be committed with a misleading description, if at all possible. Even less so several such patches in one go. Jan
