Re: MISRA C meeting tomorrow, was: MOVING COMMUNITY CALL Call for agenda items for 9 June Community Call @ 1500 UTC
On 09.06.2022 13:17, Roberto Bagnara wrote: > On 09/06/22 09:04, Jan Beulich wrote: >> On 09.06.2022 03:20, Stefano Stabellini wrote: >>> Finally, for Rule 13.2, I updated the link to ECLAIR's results. There >>> are a lot more violations than just 4, but I don't know if they are >>> valid or false positives. >> >> I've picked just the one case in xen/common/efi/ebmalloc.c to check, >> and it says "possibly". That's because evaluation of function call >> arguments involves the calling of (in this case two) further >> functions. If those functions had side effects (which apparently the >> tool can't figure), there would indeed be a problem. >> >> The (Arm based) count of almost 10k violations is clearly a concern. >> I don't consider it even remotely reasonable to add 10k comments, no >> matter how brief, to cover all the false positives. > > Again, the MISRA approach is a preventive one. > If you have reasons you want to write > > f(g(), h()); > > then declare g() and h() as pure (or const, if they are const). > E.g.: > > #if COMPILER_SUPPORTS_PURE > #define PURE __attribute__((pure)) > #else > #define PURE > #endif > > int g(void) PURE; > int h(void) PURE; > > It's good documentation, it improves compiler diagnostics, > and it satisfies Rule 13.2. But such attributes first of all should be correct. They wouldn't be in the case I've looked at (involving two __virt_to_maddr() invocations), as the underlying va_to_par() isn't pure. Still in the normal case the sequence of calls made is irrelevant to the overall result. As to improving compiler diagnostics: It has been my experience that pure and const are largely ignored when used on inline functions. The compiler rather looks at the inline-expanded code to judge. (But it has been a couple of years back that I last checked, so things may have changed since then.) Jan
Re: MISRA C meeting tomorrow, was: MOVING COMMUNITY CALL Call for agenda items for 9 June Community Call @ 1500 UTC
On 09/06/22 09:04, Jan Beulich wrote: On 09.06.2022 03:20, Stefano Stabellini wrote: Finally, for Rule 13.2, I updated the link to ECLAIR's results. There are a lot more violations than just 4, but I don't know if they are valid or false positives. I've picked just the one case in xen/common/efi/ebmalloc.c to check, and it says "possibly". That's because evaluation of function call arguments involves the calling of (in this case two) further functions. If those functions had side effects (which apparently the tool can't figure), there would indeed be a problem. The (Arm based) count of almost 10k violations is clearly a concern. I don't consider it even remotely reasonable to add 10k comments, no matter how brief, to cover all the false positives. Again, the MISRA approach is a preventive one. If you have reasons you want to write f(g(), h()); then declare g() and h() as pure (or const, if they are const). E.g.: #if COMPILER_SUPPORTS_PURE #define PURE __attribute__((pure)) #else #define PURE #endif int g(void) PURE; int h(void) PURE; It's good documentation, it improves compiler diagnostics, and it satisfies Rule 13.2. Kind regards, Roberto
Re: MISRA C meeting tomorrow, was: MOVING COMMUNITY CALL Call for agenda items for 9 June Community Call @ 1500 UTC
On 09.06.2022 03:20, Stefano Stabellini wrote: > Finally, for Rule 13.2, I updated the link to ECLAIR's results. There > are a lot more violations than just 4, but I don't know if they are > valid or false positives. I've picked just the one case in xen/common/efi/ebmalloc.c to check, and it says "possibly". That's because evaluation of function call arguments involves the calling of (in this case two) further functions. If those functions had side effects (which apparently the tool can't figure), there would indeed be a problem. The (Arm based) count of almost 10k violations is clearly a concern. I don't consider it even remotely reasonable to add 10k comments, no matter how brief, to cover all the false positives. Jan
MISRA C meeting tomorrow, was: MOVING COMMUNITY CALL Call for agenda items for 9 June Community Call @ 1500 UTC
Hi all, Just a quick note to have another look at the spreadsheet before the meeting tomorrow. I cleared the old rules we have already discussed leaving only the ones to discuss next. A few rules are similar to the already accepted Rule 5.1 with our agreed 40 characters limit for identifiers: - Rule 5.2 - Rule 5.4 A couple of rules are about comparisons/operations between pointers to different objects: - Rule 18.1 - Rule 18.2 - Rule 18.3 In my opinion these rules are good in the general case. Things like _end - _start and other "fake objects" should be deviations. A few rules weren't clear: - Rule 13.1, the example link was wrong, I updated it - Rule 9.3, { 0 } is allowed by the rule and also { [2] = 1 } is allowed - Rule 9.4, range initializers are not considered by the rule because they are a GNU extension Finally, for Rule 13.2, I updated the link to ECLAIR's results. There are a lot more violations than just 4, but I don't know if they are valid or false positives. Looking forward to our discussion tomorrow! Cheers, Stefano