Re: SAF-x-safe rename

["andrew.cooper3@xxxxxxxxxx" <andrew.cooper3@xxxxxxxxxx>]

On 05/10/2023 8:43 am, Luca Fancellu wrote:

> > On 5 Oct 2023, at 00:46, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> >
> > Hi MISRA C working group (Jan, Roger, Andrew, Julien, Bertrand, George)
> >
> > in a recent thread Andrew pointed out that the SAF-2-safe tag is
> > confusing and requested a rename:
> > https://marc.info/?l=xen-devel&m=169634970821202
> >
> > As documented by docs/misra/documenting-violations.rst:
> >
> > - SAF-X-safe: This tag means that the next line of code contains a finding, but
> >   the non compliance to the checker is analysed and demonstrated to be safe.
> > - SAF-X-false-positive-<tool>: This tag means that the next line of code
> >   contains a finding, but the finding is a bug of the tool.
> >
> >
> > Today we have already 28 instances of SAF tags in the Xen codebase.
> >
> >
> > Andrew suggested "ANALYSIS" instead of SAF so I would imagine:
> > - ANALYSIS-X-safe
> > - ANALYSIS-X-false-positive-<tool>
> >
> > If we really want a rename I suggest to rename SAF to SAFE:
> > - SAFE-X-safe
> > - SAFE-X-false-positive-<tool>
> >
> > Or maybe MISRA:
> > - MISRA-X-safe
> > - MISRA-X-false-positive-<tool>
> >
> > But I actually prefer to keep the tag as it is today.
> >       
> We chose a generic name instead of MISRA because the tag can potentially suppress findings
> of any checker, including MISRA checker.
>
> If SAF-* is confusing, what about FUSA-* ?
>
> Anyway I’m thinking that every name we could come up will be confusing at first, improving the
> documentation would mitigate it (by improving I mean to improve the fruition of it, for example a
> Read the docs documentation has the search bar, a quick copy paste of SAF- would make the
> documenting-violations page visible.)
>     
    
    No - this is a problem *because* changing the documentation doesn't
    help.   (To be clear, updating the documentation is fine, but
    irrelevant here.)
    
    
    These are annotations in code.  They need to be:
    
    1) Short (obviously)
    2) Clear to someone who isn't you (the collective us of this group)
    reading the code.
    3) Non-intrusive, so as not to get in the way of the code.
    
    and they must be all three.  This was even a principle given at the
    start of the MISRA work that we would not be deteriorating the
    quality of the code just to comply.
    
    Point 3 is other thread about end-of-line, or block regions.  Lets
    leave that there because it's really a metadata transformation
    problem constrained by where the comments can acceptably go.
    
    
    Point 2 is the issue here, and "SAF-$N-safe" scores very highly on
    the WTF-o-meter *even* for people who know that it's something
    related to MISRA.
    
    Seriously it looks like someone couldn't spell, and everyone else
    went with it (reflects poorly on everyone else).
    
    And yes, I know it's an initialisation for something, but it's not
    even an industry standard term - it's a contraction of an
    intentionally generic phrase, with substantial irony on an early
    MISRA call where there was uncertainly between people as to what it
    even stood for.
    
    These are the thoughts running through the minds of people reading
    the code when they don't understand what they're looking at.
    
    
    Annotations for other static analysers intentionally use their own
    name so they're googleable.
    
    Guess what SAF googles for?  Sustainable Aviation Fuel, or
    Specialist Automotive Finance.
    
    Fine, lets be more specific.  How about "Xen SAF" ?  Nope...
    
    "Did you mean to search for:
    Xen SAVE Xen SAN Xen VIF Xenstaff"
    
    
    Despite many of the search results referencing patches, or rendered
    documents out of docs/, not a single one of them gets
    documenting-violations.rst in any form, where the single definition
    of this term is hiding in a paragraph which spends 90% of it's
    volume describing a monotonically increasing number.
    
    Seriously, ChatGPT would struggle to make shit this good up.
    
    
    The thing we tag with *must* be trivially recognisable as an
    analysis tag in order for others to be able to read the code. 
    Therefore, it needs to be an actual full world (hence the ANALYSIS
    suggestion), or an industry standard term (where MISRA does
    qualify).
    
    I don't exactly what it is - something else might turn out to be
    even better, but it is very important to be not this, for everyone
    else to have an easy time reading the code.
    
    
    And reasoning along that line...  What's wrong with just /* octal-ok
    */ or /* womble-permitted */ so it's also apparent in context what
    the contentious issue might be and why it might be mitigated?
    
    The mechanics behind the scenes is just a trivial text replacement,
    and the tagging scheme does not have to uniform obfuscated
    identifier for that to work.
    
    ~Andrew