Re: [PATCH 1/2] docs/misra: introduce rules.rst

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Thu, 26 May 2022, Bertrand Marquis wrote:
> > On 26 May 2022, at 11:15, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> > On 26.05.2022 11:54, Bertrand Marquis wrote:
> >>> On 26 May 2022, at 10:43, Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >>> On 26.05.2022 03:02, Stefano Stabellini wrote:
> >>>> On Wed, 25 May 2022, Julien Grall wrote:
> >>>>> On 25/05/2022 01:35, Stefano Stabellini wrote:
> >>>>>> +- Rule: Dir 4.7
> >>>>>> + - Severity: Required
> >>>>>> + - Summary: If a function returns error information then that error
> >>>>>> information shall be tested
> >>>>>> + - Link:
> >>>>>> https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_07.c
> >>>>> 
> >>>>> 
> >>>>> ... this one. We are using (void) + a comment when the return is 
> >>>>> ignored on
> >>>>> purpose. This is technically not-compliant with MISRA but the best we 
> >>>>> can do
> >>>>> in some situation.
> >>>>> 
> >>>>> With your proposed wording, we would technically have to remove them 
> >>>>> (or not
> >>>>> introduce new one). So I think we need to document that we are allowing
> >>>>> deviations so long they are commented.
> >>>> 
> >>>> Absolutely yes. All of these rules can have deviations as long as they
> >>>> make sense and they are commented. Note that we still have to work out
> >>>> a good tagging system so that ECLAIR and cppcheck can recognize the
> >>>> deviations automatically but for now saying that they need to be
> >>>> commented is sufficient I think.
> >>>> 
> >>>> So I'll add the following on top of the file:
> >>>> 
> >>>> """
> >>>> It is possible that in specific circumstances it is best not to follow a
> >>>> rule because it is not possible or because the alternative leads to
> >>>> better code quality. Those cases are called "deviations". They are
> >>>> permissible as long as they are documented with an in-code comment.
> >>>> """
> >>> 
> >>> Hmm, so you really mean in-code comments. I don't think this will scale
> >>> well (see e.g. the DCE related intended deviation), and it also goes
> >>> against the "no special casing for every static analysis tool" concern
> >>> I did voice on the call.
> >> 
> >> On this subject the idea was more to define a “xen” way to document
> >> deviations in the code and do it in a way so that we could easily 
> >> substitute
> >> the “flag” to adapt it for each analyser using tools or command line 
> >> options.
> > 
> > I think the basic scheme of something like this would want laying out
> > before doc changes like the one here actually go in, so that it's clear
> > what the action is if a new deviation needs adding for whatever reason
> > (and also allowing interested people to start contributing patches to
> > add respective annotations).
> 
> We will work on that but if we wait for everything to be solved we will
> never progress.
> I have a task on my side (ie at arm) to work on that and Luca Fancellu
> will start working on it next month.
> Now I do not think that this should block this patch, agreeing on rules does
> not mean will respect all of them in the short term so we can wait a bit as I
> definitely think that how to document violations in the code and in general
> will be a work package on its own and will require some discussion.

Right.

In general, we'll need to document these deviations and ideally they
would be documented as in-code comments because they are easier to keep
in sync with the code. But we won't be able to do that in all cases.

We'll also need a special TAG to mark the deviation. Nobody wants
multiple tagging systems for different tools (ECLAIR, cppcheck,
Coverity, etc.) We'll come up with one tagging system and introduce
conversion scripts as needed. Roberto offered to help on the call to
come up with a generic tagging system.

In some cases in-code comments for every deviation would be too verbose.
We'll want to handle it in another way. It could be a document
somewhere else, or simply disabling the Rules check in ECLAIR/cppcheck
(but that partially defeats the purpose.) We'll have to see. I think
it is going to be on a case by case basis.


In short, I don't think we have all the info and expertise to come up
with a good deviation system right now. We need to make more progress
and analize a few specific examples before we can do that. But to gain
that expertise we need to agree on a set of rules we want to follow
first, which is this patch series.


So, I think this is the best way we can start the process. We can
clarify further with the comment on top of this file, and we could even
remove the specific part about the "in-code comment" with an open-ended
statement until we come up with a clear deviation strategy. For
instance:

"""
It is possible that in specific circumstances it is best not to follow a
rule because it is not possible or because the alternative leads to
better code quality. Those cases are called "deviations". They are
permissible as long as they are documented.

The existing codebase is not 100% compliant with the rules. Some of the
violations are meant to be documented as deviations, while some others
should be fixed. Both compliance and documenting deviations on the
existing codebase is work-in-progress.
"""