Re: SAF-x-safe rename

[Jan Beulich <jbeulich@xxxxxxxx>]

On 06.10.2023 00:01, Andrew Cooper wrote:
> 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).

ANALYSIS imo gets in conflict with 1) above, considering that permitting
line length violations was already brought up with the shorter SAF-*.

>> 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?

When the numbering scheme was discussed, I was asking why a shorthand
for the issue to deal with (kind of a tag) can't be used. I don't
recall any arguments, but here we are. One issue with something like
just /* octal-ok */ is that it's not sufficiently reliably machine-
parseable; that's aiui where the desire of the SAF (or whatever else)
prefix came from.

>> 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.

Both MISRA and RULE are Misra-specific; RULE, if you mean it to be
followed by the rule number, would even be Misra version specific.

Jan