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