Re: SAF-x-safe rename

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 05/10/2023 10:38 pm, andrew.cooper3@xxxxxxxxxx wrote:
> 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.

Or, as has been pointed out to me in private, even

/* RULE-$N-safe */

would be an improvement because it's clearly related to some set of
guidelines.

~Andrew