Re: MISRA C meeting tomorrow, was: MOVING COMMUNITY CALL Call for agenda items for 9 June Community Call @ 1500 UTC

2022-06-09 Thread Jan Beulich
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

2022-06-09 Thread Roberto Bagnara

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

2022-06-09 Thread Jan Beulich
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

2022-06-08 Thread Stefano Stabellini
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