On 06.10.2023 00:01, Andrew Cooper wrote: > On 05/10/2023 10:38 pm, [email protected] wrote: >> On 05/10/2023 8:43 am, Luca Fancellu wrote: >>>> On 5 Oct 2023, at 00:46, Stefano Stabellini <[email protected]> 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
